fix(review): keep host-trusted state out of the container's writable surface - #9983
fix(review): keep host-trusted state out of the container's writable surface#9983wenshao wants to merge 53 commits into
Conversation
…surface Two instances of one root cause, both reachable on `main` today and neither opened by the sandbox — before it, the reviewed repository's commands ran as the host user with no filesystem restriction at all, so containment narrowed this reach rather than creating it. What it did not do is close it, and a `required` that claims containment should not leave host-trusted state in the directory it hands the container read-write. **Leases.** `cleanupReviewWorktreeLeases` matches a lease by session ids alone and then force-removes whatever worktree and deletes whatever branch it names. Those files lived in the review temp dir — the bind mount. Reviewed code that edits one makes another session's cleanup destroy the wrong tree; one it plants with a foreign session id is swept by nobody and wedges that PR on that machine until someone deletes the file by hand. They now live one directory over, in `.qwen/review-leases`, which nothing mounts. A lease left behind by an older build is deleted when the same target is leased again. **Gitfiles.** These cannot move — git requires `<tree>/.git` — so the check moves instead. The existing identity gates cannot see the attack and never could: the writer chooses both the gitfile's target and that target's backpointer, so the round-trip agrees; `--show-toplevel` still names the tree; no symlink is involved; and asking git for the common dir resolves THROUGH the rewritten gitfile, so that answer comes from the same hand. Location is the question that has an honest answer — a real admin entry lives under `<repo>/.git/worktrees/`, outside the mount, and a planted one has to be inside it. `restoreProbeTreeTracked` now refuses an entry that resolves inside the review temp dir. Reproduced end to end before fixing, and the reproduction is the reason the gate is where it is: a rewritten gitfile pointing at a planted admin entry whose own `commondir` names a planted repository, both under the mount, passes all three existing gates — and `git checkout --force` then runs that repository's `filter.evil.smudge` ON THE HOST. With the new gate the same shape is refused while the entry it replaced still passes. Three mutants, three reds: a gate that always admits, one that admits when the entry cannot be resolved, and one that refuses when there is no mount at all — the last would refuse every ordinary repository.
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
|
Thanks for the PR! Template looks good ✓ Problem: demonstrated, not theoretical. R19-1 and R1-3 were disclosed as not closed in #9723's review, and the description carries a build-it-by-hand reproduction in which a planted admin entry passes all three existing identity gates and executes a smudge filter on the host during Direction: aligned. Host-trusted state does not belong inside the directory the review container mounts read-write — that is the containment contract the pipeline claims — and both original findings are the same root cause, so one fix is the right call. No user-facing surface changes. Size: touches Approach: minimal and coherent — move the leases one directory over (out of the mount, lifecycle unchanged), and answer the gitfile attack with location instead of identity, since identity is chosen by the attacker on both ends of the round trip. The scope grew through the review rounds, but every addition closes a route a round named — no drive-by changes. Risk: no high-risk-path matches; no elevated risk signals. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已被证实，不是理论性的。R19-1 与 R1-3 在 #9723 的评审中已被披露为「关不掉」，描述里给出了可手工构建的复现：植入的管理条目通过全部三道现有身份门，并在 方向：对齐。宿主可信状态不应留在 review 容器以读写挂载的目录之内——这是流水线声称的容器化契约——而最初两条发现同根同源，一次修复是正确的选择。无用户可见面变更。 规模：触及 方案：最小且自洽——把 lease 移到隔壁目录（移出挂载面，生命周期不变），并用「位置」而非「身份」回答 gitfile 攻击，因为往返两端都出自攻击者之手。范围在评审轮次中有所扩大，但每次扩大都对应某轮点名的一条路径——无顺手改动。 风险：未命中高风险路径；无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRe-reviewed at the post-merge head. The approach still matches what I'd propose from the description alone — move the trusted state out of the writable surface, and answer the gitfile attack with location instead of identity — and this round's changes close the one hole the last pass found, plus several the review rounds surfaced since. The boundary critical from the last triage is fixed, and fixed the stronger of the two ways. The hand-rolled The resolver-alignment fix is the right shape. The gate asks git itself ( Coverage is now complete for the host-side writes that materialise files — which is the property that matters, since Non-blocking, for the record:
Testing evidence (this PR's own CI — the review is static, PR code is never executed here)All checks on
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The unit suite now pins the behaviour claim itself, not just the predicate: the integration fixtures plant the coherent attack shape (a rewritten gitfile naming a planted admin entry with a real smudge filter) through the production call sites and assert the canary was never written — on ubuntu CI, at this head. The author's earlier macOS-only manual reproduction is therefore no longer the only end-to-end attestation. The sandboxed A/B verification requested last round ( 中文说明代码审查（基于合并 main 之后的最新 head） 方案与我仅凭描述独立提出的思路一致——把可信状态移出可写面，用「位置」而非「身份」回答 gitfile 攻击——本轮改动堵上了上次 triage 发现的那个洞，以及此后评审轮次陆续发现的几条路径。 上一轮的关键边界问题已修复，且采用了两种修法中更强的那种。 手写的 「让 git 自己回答」的修法是正确形态。 门改为直接问 git（ 对「会落盘」的宿主侧写操作，覆盖现在是完整的——这才是关键性质，因为执行植入 非阻塞，留档：
测试证据（本 PR 自己的 CI——本次评审为静态评审，不执行 PR 代码）
单元测试套件现在钉住的是行为本身，而不只是谓词：集成夹具通过生产调用点植入自洽的攻击形态（改写后的 gitfile 指向带真实 smudge 过滤器的植入管理条目），并断言金丝雀文件从未被写出——在 ubuntu CI 上、在本 head 上。作者早先仅在 macOS 上的手工复现因此不再是唯一的端到端证据。上一轮要求的沙箱 A/B 验证（ — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — the one blocking boundary from the last pass is fixed the stronger of the two possible ways, every materialising host-side git path is now gated with fail-closed semantics, CI is fully green at this head, and the only remaining nits are a stale scope sentence in the PR body and an optional gate-level equality test. Stepping back: the problem was real and demonstrated from day one, and the design landed exactly where my independent proposal did — leases out of the mount, location instead of identity. What the iteration since then added is worth saying out loud, because it is the part a first-read misses: the revert phase is reached precisely when the earlier gates fired, the scratch reuse path is the cheaper route to the same checkout, and The two nits are named in the review above and neither blocks: update the PR body's "out of scope" sentence so it matches the diff, and consider the one-line equality test. Approving, pinned to the reviewed commit. ✅ 中文说明置信度：4/5 —— 上一轮唯一的阻塞性边界问题以两种修法中更强的那种修复完毕；所有会落盘的宿主侧 git 路径现在都有失败即拒的门；本 head 上 CI 全绿；剩下的只有两条小项——PR 正文里一句过时的范围描述，以及一个可选的门级相等测试。 退一步看：问题从第一天起就是真实且已被证明的，设计也正好落在我独立提出的方案上——把 lease 移出挂载面，用位置代替身份。此后的迭代所补充的部分值得明说，因为那是粗读容易漏掉的：revert 阶段恰在前面的门开火时才会到达；scratch 复用路径是通向同一次 checkout 的更省力路线； 两条小项已在上面的评审中点名，均不阻塞：更新 PR 正文「不在范围内」那句，使之与 diff 一致；可以补一行相等情形的测试。批准，锚定在本次评审的提交上。✅ — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
One blocking finding: the new location gate admits an admin entry that resolves to the mount root itself — the boundary term re-opens the exact attack class this PR closes. Details and the one-line fix are in my review note above. 🙏
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
Test Plan (not a blocker): src/services/review-worktree-lease.test.ts — no such file or directory; 5077 passing — this review observed 23879 passed.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：反向审计——评审时间预算不足，未能开始第 5 轮。
Test Plan（非阻断）：src/services/review-worktree-lease.test.ts — no such file or directory; 5077 passing — this review observed 23879 passed。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
|
@qwen-code /takeover stop |
qwen-code-dev-bot
removed
the
autofix/takeover
|
👋 Takeover released: the autofix loop will no longer engage this PR (an in-flight round, if any, completes its bounded work). Re-apply 中文说明👋 已释放：autofix 循环不再介入此 PR（在飞的一轮如有，将完成其有界工作）。重新打上 |
Six blockers from round 1, all verified before fixing. **The predicate failed open at both boundaries of its own question.** An entry whose realpath IS the mount root produced an empty relative path and was read as outside — while the root is exactly as writable as anything under it — and a child legitimately named `..evil-git` produced a relative path starting with the characters an escape would. Both are places a planted entry can sit. The hand-rolled arithmetic is gone; `isSubpath` already answers this correctly and is already tested. **Every gate sat behind `.git` being a file.** Reviewed code that replaces the gitfile with a `.git` DIRECTORY of its own — `rm .git && git init .` inside the mount — skipped the lot, and the restore then ran against the repository it had just made. That shape is now refused before the branch, and only under a mount: outside one, a `.git` directory is what every ordinary checkout has. **The gate was on the second host-side write, not the first.** The probe tree's own creation — `git worktree add` — resolves the repository through the REVIEW worktree's gitfile, a second rewritable pointer in the same mount, written after the build phase has already run the PR's code in the container. `worktree add` checks files out, so it executes whatever filter that pointer leads to, before anything inside the restore could fire. Both call sites now ask the same question through one helper rather than two inline copies. **The legacy-lease removal was a wedge.** `force` only swallows ENOENT, so a DIRECTORY at the old lease path threw EISDIR out of acquisition — and `mkdir .qwen/tmp/qwen-review-lease-pr-42.json` is one command inside the mount, after which every review of that PR on that machine failed, with nothing able to remove it: the rollback rethrows, the sweep skips the lease shape, and `rm -f` cannot remove a directory. **The move stranded the only automated orphan sweeper.** The review job's `if: always()` heal step swept the old path only, so a cancelled or timed-out run left a dead lease at the new one, on a runner whose workspace persists. It now sweeps both, recursively. Tests follow the same rule they were missing: the gate's production call sites are driven, not just its predicate. Every previous fixture put its tree under a bare `tmpdir()`, where `mountRootFor` answers null and the gate short-circuits before its logic runs — which is how deleting the wiring shipped green. The new fixtures sit under `.qwen/tmp` and plant a COHERENT admin entry, because an incoherent one fails earlier for a reason that has nothing to do with the gate. Seven mutants, seven reds — including passing the probe tree where the review worktree belongs.
|
六条 Critical 全部处理， R1-1：我手写了路径包含判断，两个边界都 fail-open你说得对，而且这是最不该手写的东西。 已换成仓库里既有的 R1-2：所有身份门都在「
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent 6b": running npm run test:scripts to observe the pinning test failure live (no node_modules in the review worktree).
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Test Plan (not a blocker): src/services/review-worktree-lease.test.ts — no such file or directory; 5077 passing — this review observed 23883 passed.
中文说明
未探索到全部深度（达到工具调用预算）："agent 6b"：running npm run test:scripts to observe the pinning test failure live (no node_modules in the review worktree)。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
Test Plan（非阻断）：src/services/review-worktree-lease.test.ts — no such file or directory; 5077 passing — this review observed 23883 passed。
— qwen3.8-max via Qwen Code /review (v0.22.0)
…ns the sweep **The workflow edit broke its own pinning test, and this PR's required check with it.** `scripts/tests/review-worktree-cleanup-workflow.test.js` asserts the sweep line literally, derived from `paths.ts` so a rename there fails the build rather than silently no-op-ing the sweep on the runners. Adding `-r` broke the substring. The test now pins BOTH locations — the new one where this job's own runs leave leases, the old one where a persisted workspace can still hold one — which is what the test existed to enforce and what the move should have updated in the same breath. It went unnoticed because the commit was verified with vitest alone; `npm run test:scripts` is a separate lane of the same required job. **The revert phase was a third ungated write, reached exactly when the others refuse.** `git checkout base -- …` runs against the same mounted probe tree, and its only guard was a symlink-only lstat walk that a rewritten gitfile passes untouched. Worse, a restore refusal becomes `inconclusive` without throwing and the mutation phase's catch continues on purpose "so the revert probe below still runs" — so guarding the first two writes and not this one left the route open precisely where the other two closed it. **A claim in the gate's own rationale was false.** The comment called `worktree add` "the FIRST host-side git write of this phase"; `discardWorktree` above already runs `worktree remove --force` and `worktree unlock` with the same cwd. They materialise nothing, so no filter runs — but that distinction is the entire reason the gate can sit below them, and the looser sentence invites the next maintainer to add a checkout above it. Tests follow the same rule as the code. The ADMIT path had no coverage, so a mutation mangling the parsed gitdir survived: every refusal fixture would still refuse, because a mangled target fails to resolve and the location check fails closed — while in production the same mangling refuses every healthy tree. It is asserted now, relative spelling included, since git writes that pointer relative when it can. And the revert gate is driven through the phase itself, by a fake runner that rewrites `.git` from inside the probe tree — which is who does it in production. Nine mutants, nine reds.
|
四条全部处理， R2-1：我改工作流，破坏了钉住那一行的同步测试实测确认： 加了 现在它把两个位置都钉住：新位置是本 job 自己的运行留下 lease 的地方，旧位置是持久工作区上早期构建可能还留着的地方。 漏掉的原因值得记一笔：我提交前只跑了 vitest。 R1-13：revert 是第三个未设门的写入，而且恰恰在其他门拒绝之后才到这条最要紧，你的因果链我核过了：restore 的拒绝变成 给前两个写入设门却漏掉这一个，等于在其他两处刚关上的地方把路留着。已补，并且是通过该阶段本身驱动测试的：夹具里的假运行器在探针树里改写 R2-2：我在安全门的理由里写了一句不实的话
R2-4：放行路径无覆盖，你给的变异确实能存活也成立，而且理由很刁：所有拒绝用例在「篡改解析目标」的变异下仍然会拒绝——目标被弄坏就解析不了，位置检查 fail-closed，期望的拒绝照样发生——而生产里同一个变异会拒掉每一棵健康的树。只有断言放行才能把两者分开。已补，并且覆盖相对路径拼法（git 能用相对就用相对，所以解析必须相对于树而非进程 cwd）。变异（ R2-3「按调用点逐个设门关不掉这个类」我同意这是事实描述。这一轮把第三个点补上之后，三个会检出文件的宿主写入都设了门；但我没有去做共享层，因为 九个变异，九次红。review + lease 全部套件 104 文件 5085 通过， |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI (merge_group only) and the new integration tests never ran on Windows — the red that lane carries is R3-1.
Test Plan (not a blocker): src/services/review-worktree-lease.test.ts — no such file or directory.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI (merge_group only) and the new integration tests never ran on Windows — the red that lane carries is R3-1。
Test Plan（非阻断）：src/services/review-worktree-lease.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.0)
…annot exist The three tests added for this gate are deterministically red on Windows, and the reason is this PR's own sibling: `mountRootFor` refuses every absolute Windows path, because a drive letter is a colon and the `-v` grammar has one separator. Containment is unavailable there by design, so the gates never speak, `out.json` never carries the refusal, and the assertions fail for a reason that has nothing to do with what they test. The Windows lane runs only under `merge_group` and reports as skipped on the PR page, so green here was no evidence — the first red would have landed in the queue and stalled the batch. Gated with the same named predicate the sibling suite uses, so the reason is stated once rather than repeated three times. Two more from the same review. The fake runner embedded the planted path in a JS string literal by interpolation, which a Windows path's backslashes self-mangle; it is generated through `JSON.stringify` now. And both new tests had pasted the ~22-line fixture that `scaffoldModifiedPr` already provides, leaving three copies to evolve in lock-step — the helper takes the worktree location as a parameter instead, since the location is exactly what differs and exactly why the gates fire. The three gates were re-mutated after the refactor, one at a time: three mutants, three reds.
|
两条都处理了， R3-1：Windows 腿——同一个教训我第二次踩成立，而且成因正是本 PR 的姊妹改动： 而且 Windows 腿只在 这条我在 #9723 上已经被指出过一次同类问题（那次是 同一条里的第三点也修了：假运行器把植入路径用插值塞进 JS 字符串字面量，Windows 路径的反斜杠会自我损坏——现在用 R3-2：夹具三份拷贝也成立，而且你给的处方比我原来的做法好：位置差异恰恰是门是否开火的原因，所以它属于参数而不是分叉。 重构之后重新验证参数化和平台门都可能悄悄削弱防护，所以三道门在重构后逐个重新变异（创建 / restore / revert），三个变异三次红。 review + lease 全部套件 104 文件 5085 通过； |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Test Plan (not a blocker): src/services/review-worktree-lease.test.ts — no such file or directory.
Convergence: round 4 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 2 (2 new). Findings keep coming back to the same files: packages/cli/src/commands/review/test-efficacy.integration.test.ts (findings in round 3; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查。 建议见行内评论。
Test Plan（非阻断）：src/services/review-worktree-lease.test.ts — no such file or directory。
收敛情况：第 4 轮发布了 1 条行内评论，其中 1 条是首次提出；上一轮发布了 2 条（其中 2 条首次提出）。发现反复回到同一批文件：packages/cli/src/commands/review/test-efficacy.integration.test.ts（第 3 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。本轮没有未决的 Critical，因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
Both gate tests asserted a refusal MESSAGE, and `worktreeCreateFailureDetail` serializes that message identically whether the gate throws before the write or after it. So the mutant that matters — moving `worktree add` above the gate, which is exactly what the comment beside it warns a refactor might do — checked files out through the planted pointer, ran its filters on the host, and left both tests green. Creation: the probe tree not existing is what says the spawn never ran. Revert: the planted repository now carries a real `filter.evil.smudge`, and the assertion is that its canary was never written. That is the property itself rather than a proxy for it — with the gate removed the filter EXECUTES on the host inside the suite, which is what the test now catches. The fixture had to become coherent for this to mean anything: an empty planted directory fails the checkout for reasons unrelated to the gate, so it copies the real admin entry and points `commondir` at a planted common dir, the shape the reproduction in the PR description uses.
|
你说对了：消息判据分不清顺序
创建那条：判据改成探针树不存在，这才说明 spawn 没跑过。变异（交换两行顺序）→ 红（此前存活）。 revert 那条我做得更进一步它有同样的弱点，而我发现可以给它一个直接判据而不是代理判据：让植入的库带上真的 这不是「出现了某条消息」，而是「没有发生宿主执行」——属性本身。为此夹具必须变得自洽（空目录会因为与门无关的原因让检出失败），所以它现在复制真实管理条目、把 删掉 revert 门之后，那个 smudge filter 在测试套件内真的在宿主上执行了（canary 出现）。这条测试现在抓的就是这件事。 顺带你报的 结合你前几轮披露的「review worktree 里没有 node_modules」，这应该是评审环境而不是命令本身。 review + lease 全部套件 104 文件 5085 通过。tsc / lint / prettier 干净。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent 5": executing the new/changed vitest suites to confirm green (no node_modules in the review worktree; install + build exceeds session budget).
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Test Plan (not a blocker): src/services/review-worktree-lease.test.ts — no such file or directory; 5077 passing — this review observed 24748 passed.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/lib/worktree.ts:147 — [review] Two fail-closed refusal branches of untrustedGitfile have zero test coverage; a flip to admit survives the whole suite
Convergence: round 5 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 1 (1 new). The rate of new findings is not falling. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
未探索到全部深度（达到工具调用预算）："agent 5"：executing the new/changed vitest suites to confirm green (no node_modules in the review worktree; install + build exceeds session budget)。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
Test Plan（非阻断）：src/services/review-worktree-lease.test.ts — no such file or directory; 5077 passing — this review observed 24748 passed。
收敛姿态下延后（第 5 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 5 轮发布了 2 条行内评论，其中 2 条是首次提出；上一轮发布了 1 条（其中 1 条首次提出）。新发现的产出速度没有下降。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
The dual-location lease read was one-directional: a pre-move build stays blind to leases written at the new path, so for the rollout window an old fetch-pr passes its gate over a live lease and force-removes the new session's worktree mid-run (R16-1). Mirror the lease at the legacy path after the new-path wx write wins; an honored foreign lease surfacing before the mirror backs the acquisition out instead of clobbering it. The legacy read also re-granted gate authority to the one directory reviewed code can write: a planted legacy lease naming a foreign session was a permanent denial of service (R24-2). A legacy lease is now honored only while its mtime predates LEGACY_LEASE_CUTOFF_MS (the release date of the first build carrying the move — MUST be set before shipping). Release and diagnostics see both locations (R20-1): clear removes the legacy path too (recursive, so a directory at the lease name cannot EISDIR-wedge), and the held-lease messages name the path the lease was actually found at via the new readReviewWorktreeLeaseAt. The finalizer sweeps the legacy directory only for leases content-identical to the new-path twin — the mirror is readable from inside the mount, so copied credentials prove nothing, and a plant must never drive a host-side removal. readLease lstats before opening, so a planted FIFO hangs no gate read (R24-4). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
… the mount The reuse fence keyed on inputs the sandbox's read-write mount exposes: the run epoch was the plan's mtime and the plan lives inside the mount, so reviewed code could forge the stamp and have the pipeline certify an attacker-shaped tree as the A/B base (R15-4). The stamp's second line is now a random per-run nonce persisted beside REVIEW_LEASE_DIR — created atomically by the run's first host process, adopted by its shards, unreadable and unwritable from inside the mount. The failed marker carries the same fence: a planted sha-only marker no longer settles the A/B lane into "already failed" with no build attempted (R22-1). At marker write the untracked path set the build legitimately left is recorded host-side, and reuse requires the current set to be a subset — an untracked plant dropped after the stamp (dist/cli.js, what a host-side A/B executes as the BASE side) falls through to the rebuild that sweeps it (R25-1). And when a tree THIS run built fails a state check — legitimate codegen dirt, a concurrent shard's snapshot update — the command declines busy instead of discarding the live tree another shard may be mid-A/B in; discard-and-rebuild is kept for cross-run leftovers, forges, and plants (R26-1). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…-out-of-mount # Conflicts: # packages/cli/src/commands/review/scratch-tree.test.ts # packages/cli/src/commands/review/scratch-tree.ts
mountRootFor's null was overloaded: "outside any temp dir" and "inside
one, but refused" were the same value, and every trust gate read a
refusal as "nothing to police" — measured end-to-end with a symlinked
.qwen/tmp, where a gated worktree add ran through the plant and its
smudge filter wrote its canary on the host (R26-3). The gates now ask
the containment question lexically (insideReviewTmpLexically, no
filesystem) beside mountRootFor, and where the spelling says inside and
the mount says null they refuse with a reason of their own — including
a launch directory that no longer exists at its spelling, the rename
attack's shape.
The launch-dir gate's memo made that inversion permanent: a null
produced while mountRootFor consulted the filesystem was cached TRUSTED
for the process lifetime, so one from inside the
outer mount switched the gate off and a planted filter.<x>.clean
executed as the review user (R26-2). Only the pure no-marker string
scan is memoized now; inside a review temp dir the question is re-asked
on every call, as documented.
Also: releaseWorktree's not-freed predicate keyed only on refusal, so a
probe's {status: null} (spawn failure, timeout kill, deleted cwd) read
as "no objection" and freed:true was certified over a surviving
registration and branch — the next add met "missing but already
registered" (R24-3); and its first statement read the cwd outside any
try, so a deleted cwd threw uv_cwd through the never-throws contract
(R19-4). comment-status evaluated the same trust gate twice per run and
could certify a trust state its own threads[] contradicted; the verdict
is now computed once and handed to makeGitProbe (R26-4). mountRootFor's
walk bound moves from the checkout's parent to the geometry-aware root
(a linked direct parent refused every macOS /tmp checkout, R19-3), and
its Windows refusal covers the colon-less UNC shape as a class
(R19-2).
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
… gate untrustedPointer admitted a gitfile rewritten to a SIBLING worktree's legitimate admin entry: the entry is outside the mount and is not the common dir, so both location questions passed while every host-side command through it measured and mutated the sibling tree (R23-2). The gate now carries the same backpointer round-trip the write paths always have: the entry's gitdir file must resolve back to the tree it is asked about, or the tree is refused. Only a definitive mismatch is this arm's to name — an unreadable or dangling backpointer redirects nothing, is refused by the downstream identity checks with their own reasons, and the one writer who could corrupt a backpointer to duck the mismatch cannot reach an entry outside the mount at all. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
encoding: 'utf8' maps an undecodable byte in git's byte-exact rev-parse output to U+FFFD, so a gitfile target holding an invalid UTF-8 byte was judged at the U+FFFD spelling while git acted on the original bytes: a plant under the raw-byte name inside the mount and a decoy under the U+FFFD one, symlinked to the tree's real admin entry outside it, passed every question the location gate asks — the round-trip included — while the gated write resolved through the plant. No gate may judge a spelling git never printed: a U+FFFD anywhere in the answer now counts as the question not answered, which every caller already fails closed on. The witness plants the full shape (raw-byte admin entry, U+FFFD decoy) and is itself gated to Linux: NTFS is UTF-16 and APFS rejects invalid UTF-8 names with EILSEQ, so neither the fixture nor the attack can exist there. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
The symlink arm's pruneWorktrees() surfaced only the probe's refusal, so
the third shape {out: null, status: null, refusal: null} — a spawn
failure, the timeout kill, a deleted cwd — read as a successful prune:
the arm announced "Removed ... link", wrote no stderr line, and released
the lease over a registration git never swept, and the next worktree add
met "missing but already registered" with nobody told why. Same
shape-conflation class as R19-1 at a second site. Only genuine non-zero
exits stay swallowed.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
tsc --build (unlike the vitest run) rejects the readReviewWorktreeLeaseAt delegation calling a zero-argument vi.fn with two. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Unresolved, please confirm:
- [Critical] revert-hunk.ts:900 — R15-5, an open unreplied Critical thread by qwen-code-ci-bot (comment 3913328140) that round 26's machine ledger does not carry; the round-3 budget stop prevented a mechanism trace this round, so it cannot be ruled fixe…
- [Critical] review-worktree-lease.ts:234 — R16-1 (escalated to a maintainer in round 17); the mirror landed in 56b4abd but this entry's own mechanism was not separately traced this round
- [Critical] review-worktree-lease.ts:150 — R20-1, the rollout; the mirror and cutoff landed but this entry's own mechanism was not separately traced
- [Critical] review-worktree-lease.ts:152 — R24-2, the legacy fallback; not separately traced this round
- [Critical] review-worktree-lease.ts:72 — R24-4; not separately traced this round
- [Critical] base-tree.ts:227 — R26-1, the tracked-dirt arm's repo-specific premise; the arm is present (probed: dirt declines and leaves the tree standing) but its premise was not re-traced
- [Critical] lib/git.ts:98 — R26-2, the launch-dir gate's lexical trust predicate; the memo is now keyed on the lexical scan only, but whether that closes this entry's own claim was not traced (R27-11 is a distinct, newly-found defect at the same gate)
Not reviewed: build-and-test — test_windows was skipped in CI and its suite did not run locally, so the Windows-lane exposure R26-3 and D27-2 name is unmeasured.
Not reviewed: test-efficacy probe — harnessValidated was null (the positive control never ran); all 17 revert probes were inconclusive on a missing generated file and 0 mutant and 0 hunk probes executed, so no coverage claim is made either way.
Not reviewed: historical blocker re-check — the 102 stale and 46 replied-to threads from rounds 1-25 were ruled from the recovered machine ledger rather than individually re-traced, as round 26 disclosed.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": the block was executed only on this Linux lane — no macOS or Windows host is available here, so the APFS/NTFS reasoning for the 14 platform-gated tests and the …; "agent reverse-audit (round 1)": mutation verification was not performed — proving each of the 16 witnesses goes red requires editing worktree.ts, which would leave a mutated tracked file in th…; "agent reverse-audit (round 1)": none of my planned checks were cut short, but two claims above are static traces rather than executed mutations — I did not build packages/cli or edit source …; "agent reverse-audit (round 1)": executing packages/cli/src/commands/review/lib/worktree.test.ts to observe the four changed/added tests rather than deriving their verdicts — this shared work…; "agent reverse-audit (round 2)": did not execute host-execution.canary.test.ts (the CLI suite needs the workspace dist/ built in this worktree), so finding 1's chain is verified link by lin…, and 1 more.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Not linted (tool limitation, not a blocker): .github/workflows/qwen-code-pr-review.yml — actionlint embedded-shell source mapping is not yet supported.
Test Plan (not a blocker): src/services/review-worktree-lease.test.ts — no such file or directory; 690 passing — this review observed 29118 passed.
Deferred under the convergence posture (round 27, not a blocker) — recorded, not requested in this round; 2 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
packages/cli/src/commands/review/lib/git.integration.test.ts:268 — [probe] Critical [fails-closed] [new-surface] Deleted-cwd tests remove the process cwd with no win32 skippackages/cli/src/services/review-worktree-lease.ts:562 — [probe] Critical [fails-closed] [new-surface] Two scan legs each run the ladder, so one lease can end…packages/cli/src/commands/review/base-tree.ts:135 — [probe] The --ignored half of untrackedPaths has no witness anywherepackages/cli/src/commands/review/base-tree.ts:242 — [probe] The reuse fence has no witness, and launders a tamper into…packages/cli/src/commands/review/cleanup.test.ts:341 — [review] The rewrite drops the only assertion that branch -D cannot…packages/cli/src/commands/review/cleanup.ts:714 — [probe] The caller prefixes every prune reason with a…packages/cli/src/commands/review/cleanup.ts:797 — [probe] The legacy found-at spelling is unobservable at both…packages/cli/src/commands/review/cleanup.ts:1005 — [probe] The branch leg prints 'exit null' for a could-not-run probepackages/cli/src/commands/review/comment-status.ts:444 — [probe] The untrusted branch's precedence over the…packages/cli/src/commands/review/comment-status.ts:447 — [review] The HEAD read beside the new gate still collapses every…packages/cli/src/commands/review/fetch-pr.ts:717 — [probe] The --resume untrusted-gitfile gate has no test and is…packages/cli/src/commands/review/fetch-pr.ts:729 — [review] The resume status probe blanks one of three execution…packages/cli/src/commands/review/fetch-pr.ts:1073 — [review] The step-4 refusal drops the remedy both sibling refusals…packages/cli/src/commands/review/host-execution.canary.test.ts:244 — [review] Two canary assertions are insensitive to the gate they…packages/cli/src/commands/review/lib/base-tree-trust.ts:67 — [probe] Nothing ever deletes a base-tree trust file, and cleanup…packages/cli/src/commands/review/lib/git.ts:225 — [review] The refusal channel is readable only through gitProbe;…packages/cli/src/commands/review/lib/git.ts:450 — [probe] A prune that could not run flips an already-freed release…packages/cli/src/commands/review/lib/paths.ts:62 — [probe] The docstring states the move as settled while the shipped…packages/cli/src/commands/review/lib/worktree.test.ts:385 — [probe] The walk-up-reason test's only positive assertion is…packages/cli/src/commands/review/lib/worktree.test.ts:668 — [probe] The re-shaped walk fixture leaves the backpointer…- …and 18 more (see the run report)
Convergence: round 27 posted 13 inline comment(s), 12 of them reported for the first time; the previous round posted 18 (4 new). Findings keep coming back to the same files: packages/cli/src/commands/review/base-tree.ts (findings in rounds 15, 22, 25, 26; 4 more now); packages/cli/src/services/review-worktree-lease.ts (findings in rounds 16, 20, 24; 4 more now); packages/cli/src/commands/review/lib/worktree.ts (findings in rounds 19, 23, 26; 2 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
[Critical] R27-11 launchDirRefusal() judges the string process.cwd() returns, while git()/gitOpt() spawn git with NO cwd at all — so the gate and the command it guards can resolve different directories. Node caches process.cwd() after the first read, so after an ancestor rename the cached string and the kernel's cwd disagree, and the new canary arm cannot reach the shape because it re-stands the same dentry. The fix must not violate: The memo at git.ts:100-110 records only a null verdict at a spelling with no .qwen/tmp marker, and is deliberately never written inside a mount; a fix that re-reads the cwd per call must not start memoizing a marker-carrying spelling, or the refusal stops being re-asked. Fix witness: host-execution.canary.test.ts: a third arm that renames the cwd away, plants in the kernel cwd, and stands a DECOY at the stale spelling — asserting the canary is absent. Red today.
[Critical] R27-10 The --standalone route (:596-668) returns BEFORE both gates this diff adds, leaving it the one write route in runScratchTree that resolves the repository through the review worktree's gitfile with no trust judgement — and it reports available:true with the headSha that pointer answered, to the one role that executes PR-authored instructions in the resulting tree. The fix must not violate: The standalone route's object store comes through an alternates pointer to the user's store (SKILL.md:708), so the gate must judge the WORKTREE's gitfile, not the standalone tree's own .git directory — refusing the latter is N8/R27-25's defect. Fix witness: scratch-tree.test.ts: with /.git rewritten to a planted admin entry, assert runScratchTree({standalone:true}) reports available:false rather than a plant-derived headSha.
中文说明
仅完成部分审查，审查缺口已披露。
未决，请确认：共 7 条（原文未翻译，列表见上方英文部分）。
未审查（原文为英文）：build-and-test — test_windows was skipped in CI and its suite did not run locally, so the Windows-lane exposure R26-3 and D27-2 name is unmeasured.
未审查（原文为英文）：test-efficacy probe — harnessValidated was null (the positive control never ran); all 17 revert probes were inconclusive on a missing generated file and 0 mutant and 0 hunk probes executed, so no coverage claim is made either way.
未审查（原文为英文）：historical blocker re-check — the 102 stale and 46 replied-to threads from rounds 1-25 were ruled from the recovered machine ledger rather than individually re-traced, as round 26 disclosed.
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：the block was executed only on this Linux lane — no macOS or Windows host is available here, so the APFS/NTFS reasoning for the 14 platform-gated tests and the …；"agent reverse-audit (round 1)"：mutation verification was not performed — proving each of the 16 witnesses goes red requires editing worktree.ts, which would leave a mutated tracked file in th…；"agent reverse-audit (round 1)"：none of my planned checks were cut short, but two claims above are static traces rather than executed mutations — I did not build packages/cli or edit source …；"agent reverse-audit (round 1)"：executing packages/cli/src/commands/review/lib/worktree.test.ts to observe the four changed/added tests rather than deriving their verdicts — this shared work…；"agent reverse-audit (round 2)"：did not execute host-execution.canary.test.ts (the CLI suite needs the workspace dist/ built in this worktree), so finding 1's chain is verified link by lin…，另有 1 条。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
未检查（工具限制，非阻断）：.github/workflows/qwen-code-pr-review.yml——actionlint 对 workflow 内嵌 shell 的源映射尚未支持。
Test Plan（非阻断）：src/services/review-worktree-lease.test.ts — no such file or directory; 690 passing — this review observed 29118 passed。
收敛姿态下延后（第 27 轮，非阻断）——已记录，本轮不要求修改；其中 2 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 38 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 27 轮发布了 13 条行内评论，其中 12 条是首次提出；上一轮发布了 18 条（其中 4 条首次提出）。发现反复回到同一批文件：packages/cli/src/commands/review/base-tree.ts（第 15、22、25、26 轮已出过发现，本轮又有 4 条）；packages/cli/src/services/review-worktree-lease.ts（第 16、20、24 轮已出过发现，本轮又有 4 条）；packages/cli/src/commands/review/lib/worktree.ts（第 19、23、26 轮已出过发现，本轮又有 2 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，可以避免循环反复推导同一组发现；本 PR 的评审已解析为 critical 发布下限。（仅为观察——本轮评审未因此扣留任何内容。）
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
[Critical] R27-11 launchDirRefusal() judges the string process.cwd() returns, while git()/gitOpt() spawn git with NO cwd at all — so the gate and the command it guards can resolve different directories. Node caches process.cwd() after the first read, so after an ancestor rename the cached string and the kernel's cwd disagree, and the new canary arm cannot reach the shape because it re-stands the same dentry. The fix must not violate: The memo at git.ts:100-110 records only a null verdict at a spelling with no .qwen/tmp marker, and is deliberately never written inside a mount; a fix that re-reads the cwd per call must not start memoizing a marker-carrying spelling, or the refusal stops being re-asked. Fix witness: host-execution.canary.test.ts: a third arm that renames the cwd away, plants in the kernel cwd, and stands a DECOY at the stale spelling — asserting the canary is absent. Red today.
[Critical] R27-10 The --standalone route (:596-668) returns BEFORE both gates this diff adds, leaving it the one write route in runScratchTree that resolves the repository through the review worktree's gitfile with no trust judgement — and it reports available:true with the headSha that pointer answered, to the one role that executes PR-authored instructions in the resulting tree. The fix must not violate: The standalone route's object store comes through an alternates pointer to the user's store (SKILL.md:708), so the gate must judge the WORKTREE's gitfile, not the standalone tree's own .git directory — refusing the latter is N8/R27-25's defect. Fix witness: scratch-tree.test.ts: with /.git rewritten to a planted admin entry, assert runScratchTree({standalone:true}) reports available:false rather than a plant-derived headSha.
— qwen3.8-max via Qwen Code /review (v0.23.0)
|
🤖 AutoFix ran out of time before finishing (timeout (7200000ms)) (attempt 2/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 在完成前耗尽了时间（timeout (7200000ms)）（第 2/100 次尝试）—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/34214838604 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix ran out of time before finishing (timeout (7200000ms)) (attempt 3/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 在完成前耗尽了时间（timeout (7200000ms)）（第 3/100 次尝试）—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/34228491111 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Unresolved, please confirm:
- [Critical] R27-1 base-tree.ts:275 - could not be ruled on this round: no agent re-derived the claim and the round-3 audit that would have re-traced it was stopped by the time budget.
- [Critical] R27-3 base-tree.ts:470 - could not be ruled on: the recording step now uses ls-files rather than an index-refreshing status, but the reuse fence added a git status at :302 whose provenance this round could not settle.
- [Critical] R27-4 fetch-pr.ts:1073 - could not be ruled on: the step-4 gate does ask only about the launch directory (confirmed), but whether that is the defect the entry names could not be traced this round.
- [Critical] R19-2 lib/worktree.ts:397 - could not be ruled on: the fail-closed arm moved to :582 and is ruled there as R26-3; whether this entry names a distinct mechanism could not be separated from it.
- [Critical] R27-10 (body) launchDirRefusal judging the string process.cwd() returns - could not be ruled on at the production level; the related cached-cwd mechanism was confirmed only in a test arm (host-execution.canary.test.ts:508).
Not reviewed: reverse audit - stopped before round 3 by the review time budget — the agent returned no evidence of its walk twice.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": none — but for honesty about reach: I did not execute the suite, so all three findings rest on reading the fixture against lib/worktree.ts , lib/git.ts , and ….
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Not linted (tool limitation, not a blocker): .github/workflows/qwen-code-pr-review.yml — actionlint embedded-shell source mapping is not yet supported.
Test Plan (not a blocker): src/services/review-worktree-lease.test.ts — no such file or directory; 690 passing — this review observed 29124 passed.
5 fails-closed, new-surface Critical(s) were drafted inline past the resolved critical posting floor; the CLI moved them into the deferral list below (floor enforcement).
Deferred under the convergence posture (round 28, not a blocker) — recorded, not requested in this round; 5 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
packages/cli/src/commands/review/base-tree.ts:302 — [review] Critical [fails-closed] [new-surface] R28-3: The pipeline's own A/B rerun leaves junit.xml in the base tree, so the untracked-subset arm trips permanently after the first test-del…packages/cli/src/commands/review/lib/base-tree-trust.test.ts:72 — [review] Critical [fails-closed] [new-surface] R28-1: The new trust-store test's "re-keys when the plan is TOUCHED" probe depends on the filesystem's timestamp clock advancin…packages/cli/src/services/review-worktree-lease.ts:104 — [review] Critical [fails-closed] [new-surface] R27-7: Still standing: the cutoff is a future placeholder (2026-09-15) against a HEAD dated 2026-09-08, so until that date a lease-shape…packages/cli/src/services/review-worktree-lease.ts:271 — [review] Critical [fails-closed] [new-surface] R27-8: Still standing: the mirror's fallback write is the one host-side write into the mounted directory that acquisition cannot survive…packages/cli/src/services/review-worktree-lease.ts:538 — [review] Critical [fails-closed] [new-surface] R27-9: Still standing: the finalizer's single try wraps both sweep legs and the untrusted .qwen/tmp leg runs FIRST, so any throw while s…
Convergence: round 28 posted 15 inline comment(s), 10 of them reported for the first time; the previous round posted 13 (12 new). Findings keep coming back to the same files: packages/cli/src/commands/review/base-tree.ts (findings in rounds 22, 27; 2 more now); packages/cli/src/commands/review/lib/worktree.ts (findings in rounds 19, 26, 27; 1 more now); packages/cli/src/commands/review/lib/base-tree-trust.ts (findings in round 15; 1 more now), and 1 more file(s). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
[Critical] R27-11 The --standalone route in scratch-tree.ts (:596-668) returns BEFORE both gates this diff adds, leaving the one write path whose input is untrusted as the one path that never asks untrustedGitfile(worktree); it resolves the checked-out head, the object-store path and the new tree alternates target through that unjudged pointer and returns available:true. (Unanchorable: the cited lines are unchanged code outside every hunk of that file, so no inline anchor exists.) The fix must not violate: scratch-tree.ts:723-728 - 'Not a refusal, though: an unusable leftover is exactly what the rebuild exists for'; the standalone route has no rebuild to fall through to, so its failure semantics must be stated rather than copied. Please pin this with a test: a scratch-tree.test.ts arm that plants the worktree pointer and runs the --standalone route, asserting available:false or a disclosed refusal rather than a certified tree; removing the hoisted ask must turn it red.
中文说明
仅完成部分审查，审查缺口已披露。
未决，请确认：共 5 条（原文未翻译，列表见上方英文部分）。
未审查：reverse audit - stopped before round 3 by the review time budget——该 agent 连续两次未返回任何检查过程的证据。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：none — but for honesty about reach: I did not execute the suite, so all three findings rest on reading the fixture against lib/worktree.ts , lib/git.ts , and …。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
未检查（工具限制，非阻断）：.github/workflows/qwen-code-pr-review.yml——actionlint 对 workflow 内嵌 shell 的源映射尚未支持。
Test Plan（非阻断）：src/services/review-worktree-lease.test.ts — no such file or directory; 690 passing — this review observed 29124 passed。
5 条 fails-closed 且 new-surface 的 Critical 在已解析的 critical 发布下限之外被起草为行内评论；CLI 已将其移入下方延后清单（下限强制执行）。
收敛姿态下延后（第 28 轮，非阻断）——已记录，本轮不要求修改；其中 5 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 5 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 28 轮发布了 15 条行内评论，其中 10 条是首次提出；上一轮发布了 13 条（其中 12 条首次提出）。发现反复回到同一批文件：packages/cli/src/commands/review/base-tree.ts（第 22、27 轮已出过发现，本轮又有 2 条）；packages/cli/src/commands/review/lib/worktree.ts（第 19、26、27 轮已出过发现，本轮又有 1 条）；packages/cli/src/commands/review/lib/base-tree-trust.ts（第 15 轮已出过发现，本轮又有 1 条），另有 1 个文件。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
[Critical] R27-11 The --standalone route in scratch-tree.ts (:596-668) returns BEFORE both gates this diff adds, leaving the one write path whose input is untrusted as the one path that never asks untrustedGitfile(worktree); it resolves the checked-out head, the object-store path and the new tree alternates target through that unjudged pointer and returns available:true. (Unanchorable: the cited lines are unchanged code outside every hunk of that file, so no inline anchor exists.) The fix must not violate: scratch-tree.ts:723-728 - 'Not a refusal, though: an unusable leftover is exactly what the rebuild exists for'; the standalone route has no rebuild to fall through to, so its failure semantics must be stated rather than copied. Please pin this with a test: a scratch-tree.test.ts arm that plants the worktree pointer and runs the --standalone route, asserting available:false or a disclosed refusal rather than a certified tree; removing the hoisted ask must turn it red.
— qwen3.8-max via Qwen Code /review (v0.23.0)
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 candidate finding(s) this round's reviewers re-derived matched entries already carried on this PR and were set aside before verification (R28-9) — a matched posted finding is ruled in the previous-round status as always, and a matched deferral stays on the standing deferral record.
Unresolved, please confirm:
- [Critical] R27-5
packages/cli/src/commands/review/lib/worktree.ts:188— cannot tell. The round-28 marker carries only a truncated title ('Still standing: the geometry-aware walk bound stops the false-refusal class one …') and its full body could not…
Not reviewed: reverse audit — the round-1/round-2 convergence pair BOTH reported findings, so the loop never reached two consecutive dry rounds; round 3 was not run because an auditor's probe destroyed the review worktree mid-loop and the remaining time budget was reserved for verifying the audit's 38 new findings, composing and submitting.
Not reviewed: reverse audit round 2, chunks 10-22 — 13 of 22 per-chunk auditors never launched: the review worktree was deleted mid-wave by an auditor probe that renamed .qwen inside it, so their launches were refused and those territories got only their round-1 audit.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite could not run locally; it is the lane that would have exercised this diff's platform-gated tests (R28-1, R28-2, R28-3, R28-4, R29-4).
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and its suite could not run locally; it is the case-folding lane that R29-26's unwitnessed realpathSync.native canonicaliser depends on.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; Agent 7 ran the packages/cli unit suite only.
Not reviewed: verification depth — the review worktree was destroyed mid-round and restored by checkout only, so node_modules and every built dist were lost; no vitest, tsc or scratch-tree probe could run after that point, and the findings verified in that window carry witness: not run lines naming the capability that came closest.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": whether packages/cli's vitest config actually includes the new host-execution.canary.test.ts (i.e. which lanes execute the file my chunk adds) — the review wo…; "agent reverse-audit (round 2)": running the new canary suite to see whether it is among the 14 failing packages/cli tests the pre-confirmed entry counts but does not enumerate — npx vitest …; "agent reverse-audit (round 2)": verifying untrustedRepositoryFrom / insideReviewTmpLexically in packages/cli/src/commands/review/lib/worktree.ts — the gate pruneWorktrees ' new refusal …; "agent reverse-audit (round 2)": could not determine libuv's Windows st_ctim source ( FILE_BASIC_INFORMATION.ChangeTime vs LastWriteTime vs creation time) — no Windows lane available and n…; "agent reverse-audit (round 2)": could not read the reviewed commit's base-tree.ts , paths.ts , or ci.yml ( git show HEAD:<path> impossible — the worktree directory is gone and the shell c…, and 18 more.
Not reviewed: the counter-frame audit — no record shows its brief reaching an agent, so this dimension was reviewed, if at all, from a prompt the run wrote for itself.
Not linted (tool limitation, not a blocker): .github/workflows/qwen-code-pr-review.yml — actionlint embedded-shell source mapping is not yet supported.
Test Plan (not a blocker): src/services/review-worktree-lease.test.ts — no such file or directory; 690 passing — this review observed 29357 passed.
Deferred under the convergence posture (round 29, not a blocker) — recorded, not requested in this round; 1 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
packages/cli/src/commands/review/revert-hunk.test.ts:123 — [probe] Critical [fails-closed] [new-surface] R29-4 new scratchTreeFixture omits the repo-local core.autocrlf=false pin its ~30 siblings set, so both new R24-1 tests fail determinis…packages/cli/src/commands/review/base-tree.ts:161 — [review] The --ignored arm of untrackedPaths has no testpackages/cli/src/commands/review/cleanup.test.ts:133 — [review] The mock synthesizes holder.path, so nothing pins the…packages/cli/src/commands/review/cleanup.ts:714 — [review] pruneWorktrees conflates a refusal with a spawn failurepackages/cli/src/commands/review/comment-status.ts:445 — [review] A launch-dir refusal is still reported as an absent worktreepackages/cli/src/commands/review/comment-status.ts:549 — [review] The untrusted-worktree warning branch and its ordering are…packages/cli/src/commands/review/comment-status.ts:633 — [review] The degraded report certifies the opposite of what happenedpackages/cli/src/commands/review/fetch-pr.ts:717 — [review] The --resume gate is untested and discards the reason it…packages/cli/src/commands/review/fetch-pr.ts:727 — [review] The fsmonitor pin does not make the resume status…packages/cli/src/commands/review/fetch-pr.ts:918 — [review] The found-at lease path is never asserted at this call sitepackages/cli/src/commands/review/fetch-pr.ts:1076 — [review] Nothing pins which path the step-4 gate asks aboutpackages/cli/src/commands/review/host-execution.canary.test.ts:149 — [review] The negative control is not the command worktreeResidue runspackages/cli/src/commands/review/host-execution.canary.test.ts:331 — [review] The canary's base-tree arm cannot fail when its gate is…packages/cli/src/commands/review/host-execution.canary.test.ts:489 — [review] The rename test cannot witness the memo key it namespackages/cli/src/commands/review/lib/base-tree-trust.ts:97 — [review] Trust files are never swept and grow without boundpackages/cli/src/commands/review/lib/git.integration.test.ts:279 — [review] Two more cwd-deleted tests are ungated on win32packages/cli/src/commands/review/lib/git.ts:99 — [review] The launch-dir memo buys nothing and costs a load-bearing…packages/cli/src/commands/review/lib/git.ts:500 — [review] Two of the four gated wrappers have no poisoned-launch-dir…packages/cli/src/commands/review/lib/test-utils.ts:204 — [review] The shared oracle documents a screen that does not existpackages/cli/src/commands/review/lib/test-utils.ts:221 — [review] git config consumes the quotes in the planted filter value- …and 9 more (see the run report)
Convergence: round 29 posted 12 inline comment(s), 5 of them reported for the first time; the previous round posted 15 (10 new). Findings keep coming back to the same files: packages/cli/src/commands/review/base-tree.ts (findings in rounds 15, 27, 28; 1 more now); packages/cli/src/commands/review/lib/base-tree-trust.ts (findings in rounds 22, 28; 1 more now); packages/cli/src/services/review-worktree-lease.ts (findings in rounds 27, 28; 1 more now), and 1 more file(s). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
[Critical] R28-7 [certifies-falsely] [regression] lib/git.ts:450 (still standing; could not be anchored — the cited line is unchanged code outside this PR's hunks): releaseWorktree reports freed:false with a reason asserting the registration and branch survived, when worktree remove did not run to completion even though the worktree prune that follows succeeded — and the prune is the call that actually clears the registration. With removed.status === 0 and pruned.status === null (the 120s GIT_TIMEOUT_MS kill on a prune that hangs after a fast successful remove, or a cwd deleted between the two probes), couldNotRun becomes true and the result asserts 'the next git worktree add over the path will still fail with missing but already registered' when the successful remove already cleared the admin entry. cleanup.ts:907-910 then sets failedDestruction, which gates clearReviewWorktreeLease at :1133-1135, so the lease is HELD over a path that is gone and unregistered and every later cleanup of that target skips. Its sibling arms are wrong the other way: the refusal / could-not-run verdict is gated on existed, so in the registered-but-missing case releaseWorktree's own docstring exists for, a prune that never ran produces {existed:false, freed:false, reason:undefined} and both call sites (cleanup.ts:903-910, fetch-pr.ts:633-638) print nothing. Witness: throwaway repo, git 2.43.0 — after a successful worktree remove, ls .git/worktrees/ -> No such file or directory, git worktree add over the same path WITHOUT prune -> status 0, git branch -D <the removed worktree's branch> -> status 0; and in the registered-but-missing state with the prune never run (exit 127), the next worktree add at the same path and branch -> fatal: 'qwen-review/pr-9' is already used by worktree at ... exit 128, with worktree list showing it prunable. Fix: make pruned decisive wherever worktree remove did not succeed, and stop gating the refusal / could-not-run terms on existed so the registered-but-missing case reports its reason. Constraint: releaseWorktree's docstring states git worktree prune is the only thing that clears the registration for a registered-but-missing tree (git.ts:302-324), so pruned.status === null must stay decisive wherever remove did not run or did not succeed; and git.test.ts:27-35 pins worktreeReleaseResult(false, false) -> reason: undefined, which the fix must extend rather than contradict. Fix witness: a git.integration.test.ts case with a symlink or a registered-but-missing tree plus an unrunnable git, asserting {freed:false, reason} rather than a silent clean run — red while the terms are gated on existed.
[Critical] R27-11 [certifies-falsely] [new-surface] scratch-tree.ts:596 (still standing; could not be anchored — the --standalone return path is unchanged code outside this PR's hunks): the --standalone route returns BEFORE both gates this diff adds, and buildStandaloneTree still resolves the repository through the review worktree's gitfile — gitOut(worktree,'rev-parse','--path-format=absolute','--git-path','objects') and --show-object-format — then welds those answers into the new tree (objects/info/alternates, git init --object-format=) and reports available: true certifying 'a STANDALONE repository checked out at '. With a planted admin entry the object store lands inside the read-write mount, where the reviewed code can empty it; --show-toplevel still equals the worktree so the :543-551 guard does not catch it, and rev-parse HEAD still answers the pinned sha. In the common-dir rewrite shape it answers the MAIN head, which the :583 pin catches only when --fetched-sha is present — and agent-prompt.ts:1849 omits it whenever fetchedShaOf(report) is undefined. The pre-existing rationale in the args.standalone comment argues only that the route runs no checkout or status in the shared worktree; it says nothing about three rev-parse answers being welded into a new tree's object store, so the defence does not extend to this state. Witness: real untrustedGitfile at HEAD plus real git 2.43.0 — SHAPE A (plantAdminEntry into .qwen/tmp): untrustedGitfile(worktree) -> '...resolves to an admin entry inside the review temp dir...' (the gate the route returns BEFORE), guard --show-toplevel === worktree -> true, rev-parse HEAD === pinned PR head -> true, --git-path objects -> <repo>/.qwen/tmp/.evil-common/objects, object store INSIDE the read-write mount -> true, and after the reviewed code empties that store git log --oneline -1 -> FAILED: fatal: bad object HEAD while rev-parse HEAD still answers; SHAPE B (gitfile -> <repo>/.git): rev-parse HEAD === MAIN head -> true. HONEST baseline: object store outside the mount, git log works. Fix: ask the gate on the review worktree ABOVE the if (args.standalone) branch, since the route returns early. Constraint: gating the standalone TREE itself does not work — measured, untrustedGitfile(standalone tree) refuses every legitimate one because its .git is a directory the pipeline git init-ed, not the gitfile the gate expects. Fix witness: a scratch-tree.test.ts case that plants an admin entry at the review worktree and asserts the --standalone route refuses and welds no object store from the plant — red while the route returns before both gates.
中文说明
仅完成部分审查，审查缺口已披露。
本轮评审重新推导出的 1 条候选发现与本 PR 已携带的条目匹配，已在验证前搁置（R28-9）——被匹配的已发布条目照常在上一轮状态区裁定，被匹配的延后条目仍保留在延后清单记录中。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未审查（原文为英文）：reverse audit — the round-1/round-2 convergence pair BOTH reported findings, so the loop never reached two consecutive dry rounds; round 3 was not run because an auditor's probe destroyed the review worktree mid-loop and the remaining time budget was reserved for verifying the audit's 38 new findings, composing and submitting.
未审查（原文为英文）：reverse audit round 2, chunks 10-22 — 13 of 22 per-chunk auditors never launched: the review worktree was deleted mid-wave by an auditor probe that renamed .qwen inside it, so their launches were refused and those territories got only their round-1 audit.
未审查（原文为英文）：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite could not run locally; it is the lane that would have exercised this diff's platform-gated tests (R28-1, R28-2, R28-3, R28-4, R29-4).
未审查（原文为英文）：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and its suite could not run locally; it is the case-folding lane that R29-26's unwitnessed realpathSync.native canonicaliser depends on.
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; Agent 7 ran the packages/cli unit suite only.
未审查（原文为英文）：verification depth — the review worktree was destroyed mid-round and restored by checkout only, so node_modules and every built dist were lost; no vitest, tsc or scratch-tree probe could run after that point, and the findings verified in that window carry witness: not run lines naming the capability that came closest.
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：whether packages/cli's vitest config actually includes the new host-execution.canary.test.ts (i.e. which lanes execute the file my chunk adds) — the review wo…；"agent reverse-audit (round 2)"：running the new canary suite to see whether it is among the 14 failing packages/cli tests the pre-confirmed entry counts but does not enumerate — npx vitest …；"agent reverse-audit (round 2)"：verifying untrustedRepositoryFrom / insideReviewTmpLexically in packages/cli/src/commands/review/lib/worktree.ts — the gate pruneWorktrees ' new refusal …；"agent reverse-audit (round 2)"：could not determine libuv's Windows st_ctim source ( FILE_BASIC_INFORMATION.ChangeTime vs LastWriteTime vs creation time) — no Windows lane available and n…；"agent reverse-audit (round 2)"：could not read the reviewed commit's base-tree.ts , paths.ts , or ci.yml ( git show HEAD:<path> impossible — the worktree directory is gone and the shell c…，另有 18 条。
未审查：反框架审计——没有记录表明它的 brief 到达过任何 agent，这个维度即便被审查过，也是基于这次 run 自行编写的 prompt。
未检查（工具限制，非阻断）：.github/workflows/qwen-code-pr-review.yml——actionlint 对 workflow 内嵌 shell 的源映射尚未支持。
Test Plan（非阻断）：src/services/review-worktree-lease.test.ts — no such file or directory; 690 passing — this review observed 29357 passed。
收敛姿态下延后（第 29 轮，非阻断）——已记录，本轮不要求修改；其中 1 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 29 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 29 轮发布了 12 条行内评论，其中 5 条是首次提出；上一轮发布了 15 条（其中 10 条首次提出）。发现反复回到同一批文件：packages/cli/src/commands/review/base-tree.ts（第 15、27、28 轮已出过发现，本轮又有 1 条）；packages/cli/src/commands/review/lib/base-tree-trust.ts（第 22、28 轮已出过发现，本轮又有 1 条）；packages/cli/src/services/review-worktree-lease.ts（第 27、28 轮已出过发现，本轮又有 1 条），另有 1 个文件。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
[Critical] R28-7 [certifies-falsely] [regression] lib/git.ts:450 (still standing; could not be anchored — the cited line is unchanged code outside this PR's hunks): releaseWorktree reports freed:false with a reason asserting the registration and branch survived, when worktree remove did not run to completion even though the worktree prune that follows succeeded — and the prune is the call that actually clears the registration. With removed.status === 0 and pruned.status === null (the 120s GIT_TIMEOUT_MS kill on a prune that hangs after a fast successful remove, or a cwd deleted between the two probes), couldNotRun becomes true and the result asserts 'the next git worktree add over the path will still fail with missing but already registered' when the successful remove already cleared the admin entry. cleanup.ts:907-910 then sets failedDestruction, which gates clearReviewWorktreeLease at :1133-1135, so the lease is HELD over a path that is gone and unregistered and every later cleanup of that target skips. Its sibling arms are wrong the other way: the refusal / could-not-run verdict is gated on existed, so in the registered-but-missing case releaseWorktree's own docstring exists for, a prune that never ran produces {existed:false, freed:false, reason:undefined} and both call sites (cleanup.ts:903-910, fetch-pr.ts:633-638) print nothing. Witness: throwaway repo, git 2.43.0 — after a successful worktree remove, ls .git/worktrees/ -> No such file or directory, git worktree add over the same path WITHOUT prune -> status 0, git branch -D <the removed worktree's branch> -> status 0; and in the registered-but-missing state with the prune never run (exit 127), the next worktree add at the same path and branch -> fatal: 'qwen-review/pr-9' is already used by worktree at ... exit 128, with worktree list showing it prunable. Fix: make pruned decisive wherever worktree remove did not succeed, and stop gating the refusal / could-not-run terms on existed so the registered-but-missing case reports its reason. Constraint: releaseWorktree's docstring states git worktree prune is the only thing that clears the registration for a registered-but-missing tree (git.ts:302-324), so pruned.status === null must stay decisive wherever remove did not run or did not succeed; and git.test.ts:27-35 pins worktreeReleaseResult(false, false) -> reason: undefined, which the fix must extend rather than contradict. Fix witness: a git.integration.test.ts case with a symlink or a registered-but-missing tree plus an unrunnable git, asserting {freed:false, reason} rather than a silent clean run — red while the terms are gated on existed.
[Critical] R27-11 [certifies-falsely] [new-surface] scratch-tree.ts:596 (still standing; could not be anchored — the --standalone return path is unchanged code outside this PR's hunks): the --standalone route returns BEFORE both gates this diff adds, and buildStandaloneTree still resolves the repository through the review worktree's gitfile — gitOut(worktree,'rev-parse','--path-format=absolute','--git-path','objects') and --show-object-format — then welds those answers into the new tree (objects/info/alternates, git init --object-format=) and reports available: true certifying 'a STANDALONE repository checked out at '. With a planted admin entry the object store lands inside the read-write mount, where the reviewed code can empty it; --show-toplevel still equals the worktree so the :543-551 guard does not catch it, and rev-parse HEAD still answers the pinned sha. In the common-dir rewrite shape it answers the MAIN head, which the :583 pin catches only when --fetched-sha is present — and agent-prompt.ts:1849 omits it whenever fetchedShaOf(report) is undefined. The pre-existing rationale in the args.standalone comment argues only that the route runs no checkout or status in the shared worktree; it says nothing about three rev-parse answers being welded into a new tree's object store, so the defence does not extend to this state. Witness: real untrustedGitfile at HEAD plus real git 2.43.0 — SHAPE A (plantAdminEntry into .qwen/tmp): untrustedGitfile(worktree) -> '...resolves to an admin entry inside the review temp dir...' (the gate the route returns BEFORE), guard --show-toplevel === worktree -> true, rev-parse HEAD === pinned PR head -> true, --git-path objects -> <repo>/.qwen/tmp/.evil-common/objects, object store INSIDE the read-write mount -> true, and after the reviewed code empties that store git log --oneline -1 -> FAILED: fatal: bad object HEAD while rev-parse HEAD still answers; SHAPE B (gitfile -> <repo>/.git): rev-parse HEAD === MAIN head -> true. HONEST baseline: object store outside the mount, git log works. Fix: ask the gate on the review worktree ABOVE the if (args.standalone) branch, since the route returns early. Constraint: gating the standalone TREE itself does not work — measured, untrustedGitfile(standalone tree) refuses every legitimate one because its .git is a directory the pipeline git init-ed, not the gitfile the gate expects. Fix witness: a scratch-tree.test.ts case that plants an admin entry at the review worktree and asserts the --standalone route refuses and welds no object store from the plant — red while the route returns before both gates.
— qwen3.8-max via Qwen Code /review (v0.23.0)
|
⏸️ AutoFix paused by a review convergence signal: 3 consecutive automated review round(s) signalled machine-readable non-convergence (codes observed since the last maintainer response, or the window start if none: batch-fixes, root-cause-triage) — the reviewer keeps finding new defects at a rate that is not falling while the loop keeps widening the diff, so another automatic round is unlikely to converge this PR. The loop resumes once a maintainer responds on this PR (a review or comment counts, and steers the next rounds), and pauses again if the signal persists for 3 more round(s). Alternatives: split the recurring cluster into its own PR, batch the remaining fixes into one push, comment 中文说明⏸️ AutoFix 已因评审收敛信号暂停：3 轮自动评审连续发出机器可读的不收敛信号（自上次维护者响应以来观察到的信号码；若无响应则自窗口开始：batch-fixes, root-cause-triage）——评审仍在以不降的速率发现新缺陷，而循环在继续扩大 diff，再跑一轮自动修复难以收敛本 PR。维护者在本 PR 上作出回应后循环自动恢复（评论或评审均可，并将作为后续轮次的指引）；若信号再持续 3 轮会再次暂停。可选做法：把反复出问题的簇拆成独立 PR、把剩余修复攒成一批一次推送、评论 |
Four follow-ups from an adversarial audit of the fence: The untracked record was git status's collapsed path set: dist/ and node_modules/ recorded as one entry each, so a plant dropped INSIDE a directory the build left — the exact executable a host-side A/B runs — changed no membership and passed the subset fence. The record is now file-level (ls-files --others, which never collapses), so an addition inside a recorded directory IS a membership change. The trust file was keyed by the plan's path and mtime, and the plan lives inside the mount: a utimensat backdate to an earlier run's exact stamp re-keyed this run to that run's file, adopting its nonce. The key now takes max(mtimeMs, ctimeMs) — ctime cannot be set from userland, so the identity cannot be rolled back. The busy arm over-generalized: a rewritten pointer or moved HEAD on a tree this run built has no benign cause, and declining left the plant standing for the run. Those arms now fall through to discard-and-rebuild — while tracked dirt AND untracked additions (an A/B's own snapshot --update or cache output is the benign shape) decline busy instead. And the discard itself could not rebuild a planted tree: worktree remove refuses a tree whose gitfile no longer points back, so the removal cleared the coherent plant's admin dir while the real registration survived and the next add met "missing but already registered" — the deferred refusal->rebuild wedge. When the remove fails, the reverse scan now also runs, narrowed to the tree's own basename so a tampered sibling entry is never deleted. Also pins the sweep's per-field equality with a same-target doctored twin (the worktreePath comparison had no witness) and gives readReviewWorktreeLeaseAt a legacy found-at-path witness.
…ew-host-state-out-of-mount
af7b44f to
414557e
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Critical] Blocking finding(s) follow.
Partially reviewed — gaps disclosed.
Unresolved, please confirm:
- [Critical] R29-2
base-tree.test.ts:292— the round-29 claim that the production-geometry forge test pins only the stamp forge: this round's verifier shard for it failed to execute, so the mechanism could not be re-traced at the reviewed commit. - [Critical] R27-2
base-tree.ts:304— the untracked fence checking path MEMBERSHIP only: this round confirmed a different defect at the same line (the O(n^2) subset scan) and could not rule the membership-only claim. - [Critical] R15-4
base-tree.ts:501— two of the four entrances R15-4 named still completing unchanged: not re-traced this round; the verifier shard owning it failed to execute. - [Critical] R29-4
lib/base-tree-trust.ts:90— the trust root derived by walking two directories up from a caller-supplied path: not re-traced this round. - [Critical] R28-3
lib/worktree.test.ts:526— theworktreeResiduecases assertingunmeasured: this round found a different defect at :527-529 (the win32 gate) and could not rule the original assertion claim. - [Critical] R29-5
services/review-worktree-lease.ts:222— the legacy mirror documented as an advisory write that hands gate authority: the related mechanism at :328 was confirmed only at low confidence this round, so the recorded blocker cannot be ru… - [Critical] R27-11
scratch-tree.ts:596(body) — the--standaloneroute: this round confirmed the shape at low confidence only (revert-hunk.ts:897refuses the.git-directory tree--standalonebuilds), which is terminal-only and cannot rule the…
Not reviewed: verification of reverse-audit findings g09-g16 (base-tree-trust.test.ts, lib/paths.ts nested geometry, lib/git.ts launch-dir memo, worktree.test.ts win32/U+FFFD/adminEntry cases) — the verifier shard failed to execute and was not relaunched before the deadline.
Not explored to full depth (tool budget reached): "agent 1c": whether any command rewrites the plan file after a base tree is built within one run (which would move runKeyMs and re-key the trust file, forcing a discard+r…; "agent reverse-audit (round 1)": whether the test_windows lane executes test-efficacy.integration.test.ts 's ungated cases (e.g. :449 'probes in a disposable worktree and never mutates the…; "agent reverse-audit (round 2)": whether gate 1's neutralized run actually completed git worktree add through the planted pointer ( created = true , files checked out) or failed at the add — …; "agent reverse-audit (round 2)": mutation-probing the lease file's new witnesses (the node:fs plant test and the two new describes) — verified green as written and reasoned to discriminate, n…; "agent reverse-audit (round 2)": no Windows/macOS lane run of the changed lease tests; portability was assessed by reading only ( utimesSync , rmSync recursive, join -derived paths on both s…, and 1 more.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
— [unverified] tag when the loop ended — the verifier never ruled on them, and they are not confirmed.
Not linted (tool limitation, not a blocker): .github/workflows/qwen-code-pr-review.yml — actionlint embedded-shell source mapping is not yet supported.
Test Plan (not a blocker): src/services/review-worktree-lease.test.ts — no such file or directory; 690 passing — this review observed 29358 passed.
Deferred under the convergence posture (round 30, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/review/base-tree.test.ts:272 — [review] R30-40 The only test named for the plan-backdating fence cannot fpackages/cli/src/commands/review/base-tree.ts:149 — [review] R30-42 The invariant that justifies choosing ls-files --others ovpackages/cli/src/commands/review/base-tree.ts:279 — [review] R29-3 A *missing* record is treated as proof the tree is not livpackages/cli/src/commands/review/base-tree.ts:302 — [review] R30-35 The subset fence is an O(n²) Array.prototype.includes scanpackages/cli/src/commands/review/cleanup.test.ts:501 — [review] R30-59 Both new symlink-arm witnesses pin the announcement and thpackages/cli/src/commands/review/cleanup.ts:714 — [review] R30-5 pruneWorktrees now returns two semantically different reaspackages/cli/src/commands/review/cleanup.ts:745 — [review] R30-6 The root captured at entry is handed only to redirectedAncpackages/cli/src/commands/review/comment-status.ts:445 — [review] R30-44 The diff splits "absent" from "unusable" for exactly one upackages/cli/src/commands/review/comment-status.ts:549 — [review] R30-34 The new untrusted-worktree warning is never asserted, andpackages/cli/src/commands/review/comment-status.ts:635 — [review] R30-45 The degraded report hardcodes worktreeUntrusted: null on tpackages/cli/src/commands/review/fetch-pr.test.ts:351 — [review] R30-7 The mock hardcodes the *new* lease directory as path, andpackages/cli/src/commands/review/fetch-pr.ts:717 — [review] R30-8 The new --resume pre-flight gate has no test anywhere in tpackages/cli/src/commands/review/fetch-pr.ts:1052 — [review] R30-46 The step‑4 ask narrows no window that the wrapper does notpackages/cli/src/commands/review/host-execution.canary.test.ts:7 — [review] R30-60 The canary file this diff adds — and the untrustedGitfilepackages/cli/src/commands/review/host-execution.canary.test.ts:240 — [review] R30-61 The arm titled "every wrapper in lib/git refuses to run frpackages/cli/src/commands/review/host-execution.canary.test.ts:331 — [review] R30-10 The base-tree arm cannot fail if the reuse-path pointer gapackages/cli/src/commands/review/lib/base-tree-trust.ts:47 — [review] R30-13 This doc states the *opposite* of the contract the producepackages/cli/src/commands/review/lib/base-tree-trust.ts:97 — [review] R30-14 Moving the state out of .qwen/tmp also moved it out of evepackages/cli/src/commands/review/lib/git.integration.test.ts:279 — [review] R30-15 Both new "cwd deleted mid-call" tests (releaseWorktree's *packages/cli/src/commands/review/lib/git.ts:158 — [review] R30-62 Two of the four launch-dir gate lines this diff adds — git- …and 25 more (see the run report)
Convergence: round 30 posted 12 inline comment(s), 10 of them reported for the first time; the previous round posted 12 (5 new). Findings keep coming back to the same files: packages/cli/src/services/review-worktree-lease.ts (findings in round 29; 4 more now); packages/cli/src/commands/review/base-tree.ts (findings in rounds 15, 27, 29; 2 more now); packages/cli/src/commands/review/lib/worktree.ts (findings in round 26; 2 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
[Critical] R26-3 R26-3 still standing: The fail-closed arm treats mountRootFor's null as "a redirect or an unmountable shape refused it", but the null this diff moved out of sandboxed-exec.ts is universal on Windows by design ("On Windows this refuses EVERY absolute path … a drive letter is a colon" — the deleted docstring), so on Windows the gate does not stay silent as its own docstring and four test comments claim, it refuses every tree under a ` (could not be anchored)
中文说明
仅完成部分审查，审查缺口已披露。
未决，请确认：共 7 条（原文未翻译，列表见上方英文部分）。
未审查（原文为英文）：verification of reverse-audit findings g09-g16 (base-tree-trust.test.ts, lib/paths.ts nested geometry, lib/git.ts launch-dir memo, worktree.test.ts win32/U+FFFD/adminEntry cases) — the verifier shard failed to execute and was not relaunched before the deadline.
未探索到全部深度（达到工具调用预算）："agent 1c"：whether any command rewrites the plan file after a base tree is built within one run (which would move runKeyMs and re-key the trust file, forcing a discard+r…；"agent reverse-audit (round 1)"：whether the test_windows lane executes test-efficacy.integration.test.ts 's ungated cases (e.g. :449 'probes in a disposable worktree and never mutates the…；"agent reverse-audit (round 2)"：whether gate 1's neutralized run actually completed git worktree add through the planted pointer ( created = true , files checked out) or failed at the add — …；"agent reverse-audit (round 2)"：mutation-probing the lease file's new witnesses (the node:fs plant test and the two new describes) — verified green as written and reasoned to discriminate, n…；"agent reverse-audit (round 2)"：no Windows/macOS lane run of the changed lease tests; portability was assessed by reading only ( utimesSync , rmSync recursive, join -derived paths on both s…，另有 1 条。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
— [unverified] 标记——验证者从未对它们作出裁决，它们不算已确认。
未检查（工具限制，非阻断）：.github/workflows/qwen-code-pr-review.yml——actionlint 对 workflow 内嵌 shell 的源映射尚未支持。
Test Plan（非阻断）：src/services/review-worktree-lease.test.ts — no such file or directory; 690 passing — this review observed 29358 passed。
收敛姿态下延后（第 30 轮，非阻断）——已记录，本轮不要求修改：共 45 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 30 轮发布了 12 条行内评论，其中 10 条是首次提出；上一轮发布了 12 条（其中 5 条首次提出）。发现反复回到同一批文件：packages/cli/src/services/review-worktree-lease.ts（第 29 轮已出过发现，本轮又有 4 条）；packages/cli/src/commands/review/base-tree.ts（第 15、27、29 轮已出过发现，本轮又有 2 条）；packages/cli/src/commands/review/lib/worktree.ts（第 26 轮已出过发现，本轮又有 2 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，可以避免循环反复推导同一组发现；本 PR 的评审已解析为 critical 发布下限。（仅为观察——本轮评审未因此扣留任何内容。）
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
[Critical] R26-3 R26-3 still standing: The fail-closed arm treats mountRootFor's null as "a redirect or an unmountable shape refused it", but the null this diff moved out of sandboxed-exec.ts is universal on Windows by design ("On Windows this refuses EVERY absolute path … a drive letter is a colon" — the deleted docstring), so on Windows the gate does not stay silent as its own docstring and four test comments claim, it refuses every tree under a ` (could not be anchored)
— qwen3.8-max via Qwen Code /review (v0.23.1)
…zer's legs Merge origin/main (the content-filter screen and CHECKOUT_INERT) into the branch, keeping this PR's untrustedGitfile gate ahead of the probe-tree add while adopting main's inert checkout flags, and address the smallest blocking subset of the open review findings: - base-tree-trust.test: the ctime discrimination probe raced the kernel's coarse timestamp tick — a chmod in the same tick as the fixture write leaves ctimeMs bit-identical, so the re-key assertion was deterministically red. Touch until the ctime observably moves, bounded by a deadline (R29-8). - review-worktree-lease: LEGACY_LEASE_CUTOFF_MS was a guessed future release date, so the bound honored plants written before it; pin it to the landing date (R30-32). - cleanupReviewWorktreeLeases: one try wrapped both scan legs, so an unreadable mounted directory suppressed the trusted directory's sweep; each leg now fails alone (R30-37).
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Autofix round — PR #9983This round ran under the round-time budget brake (two prior rounds exhausted the budget), so it lands the required base-merge conflict resolution plus the smallest blocking subset of findings, and records every remaining finding as a per-thread deferral reply. Merge with
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Unresolved, please confirm:
- [Critical] R30-6 packages/cli/src/commands/review/lib/worktree.ts:605 — the recovered round-30 ledger truncates this claim to 60 characters and this round did not re-trace its mechanism against the code, so it could not be ruled still-stands or fixed.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally, so the Windows-lane findings rest on a declared Windows model rather than a Windows run.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": none of my scope was cut short — I read the whole assigned diff range (diff lines 1541-1937, untruncated) and the surrounding source of every hunk in it.; "agent reverse-audit (round 1)": none of my checks were cut short, but two things I examined only from the post-change source rather than the diff, because they sit outside chunk 19 and I did n….
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Not linted (tool limitation, not a blocker): .github/workflows/qwen-code-pr-review.yml — actionlint embedded-shell source mapping is not yet supported.
Test Plan (not a blocker): src/services/review-worktree-lease.test.ts — no such file or directory; 690 passing — this review observed 29577 passed.
1 fails-closed, new-surface Critical(s) were drafted inline past the resolved critical posting floor; the CLI moved them into the deferral list below (floor enforcement).
Deferred under the convergence posture (round 31, not a blocker) — recorded, not requested in this round; 5 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
packages/cli/src/services/review-worktree-lease.ts:580 — [review] Critical [fails-closed] [new-surface] R30-9: R30-9 still standing: one try wraps both legs of the finalizer sweep, so a throw from the unguarded per-entry rmSync of a file in…packages/cli/src/commands/review/lib/git.integration.test.ts:267 — [probe] Critical [fails-closed] [new-surface] The three new deleted-cwd witnesses chdir into a real temp directory and rmSync the process's own cwd with no capability gate; …packages/cli/src/commands/review/revert-hunk.test.ts:833 — [probe] Critical [fails-closed] [new-surface] The new R24-1 control is a plain it whose fixture is deliberately inside the review temp root, so the gate this same hunk adds refuse…packages/cli/src/commands/review/scratch-tree.test.ts:56 — [probe] Critical [fails-closed] [new-surface] The win32 skip covers only the three new tests, but the new untrustedGitfile(worktree) gate refuses every Windows path, so the 40 ungat…packages/cli/src/services/review-worktree-lease.ts:271 — [review] Critical [fails-closed] [new-surface] Any non-EEXIST failure of the legacy mirror write, which lives in the distrusted mount, propagates out of createReviewWorktreeLease and …packages/cli/src/commands/review/base-tree.ts:161 — [review] base-tree.ts:161 — the --ignored leg of untrackedPaths, the only leg that can see a plant at a gitignored path, is unexercised by every test because no base-tree fixture repositor…packages/cli/src/commands/review/base-tree.ts:302 — [review] base-tree.ts:302 — the reuse fence subset check is O(n*m) Array.includes over the full ignored path set (9,168 ms measured against 4 ms for a Set) on the fast path whose purpose i…packages/cli/src/commands/review/cleanup.ts:706 — [review] cleanup.ts:706 — the third arm (a genuine non-zero exit stays swallowed and returns null) is asserted in the comment but has no test; mutating status === null to status !== 0 le…packages/cli/src/commands/review/cleanup.ts:714 — [review] cleanup.ts:714 — pruneWorktrees returns two structurally different reasons through one undiscriminated string while its only caller hardcodes the launch-directory prefix, so a spawn…packages/cli/src/commands/review/cleanup.ts:745 — [review] cleanup.ts:745 — the new entry guard catches only a launch directory that is already gone; the same uv_cwd throw still escapes uncaught from scratchWorktreesOf later in runCleanup (…packages/cli/src/commands/review/comment-status.ts:549 — [review] comment-status.ts:549 — the new untrusted-worktree warning branch is never asserted, only the report fields are, so swapping the branch order or dropping it keeps the suite g…packages/cli/src/commands/review/fetch-pr.ts:718 — [review] fetch-pr.ts:718 — the new --resume pointer gate and its new worktree-untrusted ResumeRefusal value are unreachable in the whole suite, yet lib/worktree.ts's docstring already claim…packages/cli/src/commands/review/fetch-pr.ts:1081 — [review] fetch-pr.ts:1081 — the step-4 rollback's routing, the substance of that hunk's second change, has no test: the only positive branch -D gitOpt assertion sits in a test that dies …packages/cli/src/commands/review/host-execution.canary.test.ts:297 — [review] host-execution.canary.test.ts:297 — the base-tree arm cannot go red when the gate it exists to pin is removed (mutation run: 1 passed, identical to baseline), bec…packages/cli/src/commands/review/lib/base-tree-trust.ts:97 — [review] base-tree-trust.ts:97 — nothing anywhere sweeps .qwen/review-leases/base-tree/, so each run identity leaves a multi-megabyte trust file forever on a persisted workspace a…packages/cli/src/commands/review/lib/git.ts:102 — [review] git.ts:102 — the trustedLaunchDir memo caches a verdict that provably costs no I/O to recompute (the only syscall, process.cwd(), is not and cannot be memoized), so it saves nothing…packages/cli/src/commands/review/lib/paths.ts:51 — [review] paths.ts:51 — the rationale for the new REVIEW_LEASE_DIR constant misstates the defenses that already exist (removeLeaseWorktree confines removal to <root>/.qwen/tmp at review-work…packages/cli/src/commands/review/lib/worktree.test.ts:2570 — [review] worktree.test.ts:2570 — the raw-byte R8-2 fixture builds a plant git rejects outright (status 128, empty stdout), so the case never reaches the U+FFFD guard it says it pi…packages/cli/src/commands/review/lib/worktree.test.ts:2703 — [review] worktree.test.ts:2703 — the PATH-shim directory is created with mkdtempSync directly and never registered in the block's cleanup list, so it leaks a directory holding an …packages/cli/src/commands/review/lib/worktree.ts:639 — [review] worktree.ts:639 — adminEntryInsideReviewTmp reads mountRoot's overloaded null as "nothing to police" and answers false (fail-open), the exact inversion the two gates added besi…- …and 10 more (see the run report)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未审查（原文为英文）：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally, so the Windows-lane findings rest on a declared Windows model rather than a Windows run.
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：none of my scope was cut short — I read the whole assigned diff range (diff lines 1541-1937, untruncated) and the surrounding source of every hunk in it.；"agent reverse-audit (round 1)"：none of my checks were cut short, but two things I examined only from the post-change source rather than the diff, because they sit outside chunk 19 and I did n…。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
未检查（工具限制，非阻断）：.github/workflows/qwen-code-pr-review.yml——actionlint 对 workflow 内嵌 shell 的源映射尚未支持。
Test Plan（非阻断）：src/services/review-worktree-lease.test.ts — no such file or directory; 690 passing — this review observed 29577 passed。
1 条 fails-closed 且 new-surface 的 Critical 在已解析的 critical 发布下限之外被起草为行内评论；CLI 已将其移入下方延后清单（下限强制执行）。
收敛姿态下延后（第 31 轮，非阻断）——已记录，本轮不要求修改；其中 5 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 30 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.23.1)
…stitution route From the adversarial audit of the takeover batch: - releaseWorktree's symlink arm now has its own witness for the status-null prune (the main arm had one; the sibling arm was pinned by nothing — the mutation survived green). - The launch-dir memo's comment now names the rename attack's actual premise — Node's cachedCwd, not a kernel property, so a future in-process chdir is the thing that re-opens the window — and the canary pins the stale-spelling premise instead of assuming it. - scratch-tree's --standalone shape asks the location question before buildStandaloneTree: headSha was read through the review worktree's own gitfile, and with no --fetched-sha pin a planted HEAD would put the plant's content in the tree presented as the commit under review. - revert-hunk's outermost-bound arithmetic gains a nested-geometry witness (a swap at the OUTER layer), so a drift to lastIndexOf reds it.
…M/qwen-code into fix/review-host-state-out-of-mount
The entry-captured repositoryRoot was handed to the ancestor walks but not to scratchWorktreesOf: its resolve(worktree) and redirectedAncestor default stop both read process.cwd() again, so a cwd deleted mid-run threw uv_cwd out of the sweep — past the entry guard that exists to catch exactly that. Both now anchor at the captured root. The witness is the call count: a full cleanup reads process.cwd() exactly once.
Rounds 27-30 found the nonce-in-marker design's hole: the markers carried the run secret in cleartext inside the tree — a child of the read-write mount — so reviewed code read it and forged a settling failed-marker (the A/B lane suppressed as "infrastructure") or an ok marker over a tree it shaped. The fence now reads the trust record alone, kept outside the mount: per tree it carries the pinned base, the build's verdict, and the untracked inventory per file with its size+ctime pair — an in-place rewrite of a recorded file is as visible as an added one. Markers in the tree are informational baseSha notes, excluded from every fence decision. The trust file is one per plan path, rotated in place on identity drift so no earlier generation survives to adopt; the run identity is the worktree lease's mtime when one is held (host-side, mount-untouchable) and the plan mtime with the ledger's tolerance otherwise. A mid-run mergeBaseSha rewrite conflicts with the pin and declines rather than rebuilding at a sha named after the fact. Same-run trees with a torn or missing record decline busy instead of being discarded; pointer/HEAD rewrites still discard-and-rebuild. The record step re-asks the pointer question first, and the git helpers carry the inert-config pins and a 512 MiB buffer so a real tree's ignored listing cannot silently drop the record.
Where containment cannot exist — a drive-letter colon, a UNC share, a colon-bearing POSIX checkout — the review never ran sandboxed, so the mount is no trust boundary and the gates' docstrings promise silence. The fail-closed arm read every null as a refusal and refused unconditionally there (measured: 43 red tests under the declared Windows model). mountRootFor now shares one implementation with a discriminated nullKind (outside / unmountable / refused); the gates fail closed only on a redirect or a root that vanished mid-check, and stay silent where containment cannot exist. The pointer gate's round-trip is now exact: the admin entry must own the tree the question is about (untrustedRepositoryFrom compares against git's own --show-toplevel, which a borrow cannot bring into agreement), an owner that cannot be resolved fails closed, and a redirect planted between the mount root and the owner is refused before anything resolves through it — the round trip must not agree with a link. The mount root is threaded through the questions rather than re-asked mid-gate, so a mid-gate flip of the mount cannot split the answer. The flat-case walk bound moves to .qwen: the checkout's own directory is the user's layout, not the pipeline's. releaseWorktree's not-freed predicate keys on the prune alone now: a remove that answered 0 already cleared the registration, and keying on the remove's null published freed:false over a release the prune had completed. Witnesses pin each arm, including the symlink arm that had none.
The mtime-bounded honored read is gone entirely: a pinned-to-the-past cutoff froze the honored population at release day while remaining a utimes-forgeable permanent DoS channel from inside the mount. A legacy-path file is residue now — the mirror still replaces it (loudly, naming the displaced session, when it parses as another session's lock) so pre-move builds keep seeing the lock, and only the new path answers the gate. The mirror write is best-effort (an obstruction in the mounted directory is mount weather, warned and skipped, never a failed acquisition) and replaces via tmp-file + renameSync, which never follows a planted symlink and never opens a readerless FIFO. The finalizer is driven solely by the trusted directory: one destructive pass per target, the legacy twin deleted only when content-identical — and never listed, so a poisoned mount cannot shape the sweep. The lease directory re-roots lexically to the outermost enclosing repository in the nested geometry, so a review launched from inside another review's worktree keeps its host-trusted state outside every layer's mount. fetch-pr's step 4 also asks the destination's ancestors: a symlink left standing by a declined releaseWorktree would otherwise have mkdirSync and worktree add create and check out the PR's code at the link's target — outside the review temp dir, unsandboxed. And the review job's lease sweep feeds both globs through remove_review_tree's repair ladder instead of a bare rm -rf, so a mode-000 wedge is reported rather than silently surviving. Tests that delete the process's own cwd or need a mount are gated to hosts where those exist.
The launch-dir gate judged Node's cached process.cwd() spelling while every wrapper spawns with no cwd, inheriting the KERNEL cwd — and the two diverge exactly once an ancestor is renamed. A leaf rename inside the mount plus the spelling re-stood as a symlink to an honest sibling made the cached spelling pass every arm while git ran in the renamed plant. The gate now asks the kernel (/bin/pwd -P) where the process stands and judges that spelling; a divergence is the rename, refused. Windows keeps the cached-spelling judgement: no /bin/pwd there, and no containment to protect either. The canary witness drives the full leaf-rename shape and goes red with the kernel judgement disabled.
4 participants
What this PR does
Keeps host-trusted state out of the directory the review sandbox bind-mounts read-write, and stops host-side git from resolving through a pointer that lives inside it.
Three parts:
.qwen/tmp(mounted read-write) to.qwen/review-leases(mounted by nothing).Why it's needed
Findings R19-1 and R1-3 from #9723's review are the same root cause: state the pipeline later trusts lives in the directory it hands the container read-write.
Neither is opened by the sandbox. On
maintoday the reviewed repository's commands run as the host user with no filesystem restriction at all, so both are reachable there and reachable more widely; containment narrowed the reach rather than creating it. What it did not do is close it, and arequiredthat claims containment should not leave host-trusted state inside the writable surface.Leases.
cleanupReviewWorktreeLeasesmatches a lease by(sessionId, promptId, repositoryRoot)alone — no provenance check — and then force-removes whatever worktree and deletes whatever branch it names. The files lived in the review temp dir. Reviewed code that edits one makes another session's cleanup destroy the wrong tree; one it plants with a foreign session id is swept by nobody and wedges that PR on that machine until an operator deletes the file by hand.Gitfiles. A pipeline tree's
.gitcannot move — git requires it at<tree>/.git— so the check moves instead. The identity gates that already exist cannot see this attack and never could: the writer chooses both the gitfile's target and that target's backpointer, so the round-trip agrees with itself;--show-toplevelstill prints the tree the gitfile sits in; no symlink is involved anywhere; and asking git for the common dir resolves through the rewritten gitfile, so that answer comes from the same hand. Location is the question that does have an honest answer: a real linked worktree's admin entry lives under<repo>/.git/worktrees/, outside the mount, and a planted one has to be inside it.Why the gate had to be asked in more places (R9-1). The first cut asked it at the writes — the checkouts a planted
filter.<x>turns into host execution — and nowhere else. A class closed one call site at a time re-opens at the next call site somebody adds, and it did:worktreeResidue(scratch-tree,agent-prompt)git status, which refreshes the index, and a refresh runs the repository'scleanfilterfetch-prsteps 1–2cleanStale's force-remove and the PRgit fetch, both discovering the repository fromprocess.cwd()process.cwd()—load-rules,submit,match-remote,run,revert-hunk,cleanupshow <base>:<path>), the submission target, the remote that gets fetchedcomment-statusgit -C <review worktree>probeschangedSinceComment: falsesourced from a repository the PR author planted — the pipeline certifying that the code behind a blocker thread did not moverepo-context--git-common-dir,cat-file -e,ls-tree,show <base>:<path>captureLocalDiffrev-parse --show-toplevel, thengit diff(index refresh;diff.<driver>.command)base-tree's reuse fast pathrev-parse HEADthrough that tree's own.git— and it returns, so the gate below the rebuild never runs for itrevert-hunkcheck-attrandls-fileson the scratch tree — no execution here,git applyruns no filter.qwen/tmponlymountRootFordocuments, a plant one layer up is outside the inner root, inside the outer review's read-write mount, and admitted by every gateThe launch-directory rows are one problem, not seven, and they are fixed as one: every wrapper in
lib/gitruns git with nocwd, soprocess.cwd()is where all of them discover their repository. The question is asked there — four functions in one file — rather than at each command that happens to run git. The throwing wrappers throw;gitProbeanswers its own documented "could not be run at all" value instead, because that is a probe's contract and everygitOptcaller already handles it.releaseWorktreeshows why that split is right: through a poisoned launch directory itsworktree removeis refused and thermSyncbehind it still clears the path, so the remedy keeps working without git acting through the plant.fetch-prkeeps its own gate on top, because it changes state that is not a git call — the lease read and write — before it makes one.The containment row is the one that needed care. The mount has to stay innermost — the first occurrence would widen the bind mount to the outer temp dir and pull
<repo>/.gitand every sibling checkout into the container. But "which directory may a container have been given read-write" and "where may reviewed code already have written" are different questions, and in a nested layout they have different answers. Only the distrust judgement widens, to the outermost layer. It costs the honest layouts nothing:git worktree addrun from inside a linked worktree still writes its admin entry under the main repository's.git/worktrees/, which is outside every layer rather than merely outside the deepest — asserted in the suite so a future git that changed this would red rather than silently refuse every nested review.mountRootFormoves fromsandboxed-execintoworktree(re-exported from its old home, so every importer is unchanged) so the gates can default to it instead of taking it as an argument each new call site has to remember to pass. That is what makes the closure structural rather than another round of per-call-site patches.Update (2026-09-08, head 582e5c8) — the round-26 standing Criticals
All 18 Criticals standing at round 26 are fixed in eight commits, each with its finding's prescribed fix-witness and a performed mutation check (remove the new arm, watch the witness go red, restore):
mountRootForand fail closed where the spelling says inside and the mount says null; the launch-dir memo is keyed on that lexical scan only; the Windows refusal covers the colon-less UNC shape; the symlink walk's bound is geometry-aware.releaseWorktreetreatsstatus: nullas not-freed and degrades instead of throwing on a deleted cwd.gitdirbackpointer withisSubpathso the launch-directory form stays correct.Re-verified three older threads while sweeping the history, and fixed two that were still open: the lossy-decode divergence (R8-2 — 3cc9638, a U+FFFD in git's answer counts as the question not answered) and the prune arm's third-shape read (R23-8 — 4747901). R23-12's mechanism is the R23-2 round-trip; its prescribed placement is deviated from deliberately — the check lives in
untrustedPointerwithisSubpathrather than root equality, which is the correct generalization for subdirectory launch directories.All 134 review threads carry a per-thread disposition reply and are resolved. Suggestion-level and deferred items (test coverage, docs) remain tracked in the review's findings artifact as follow-up work.
Rounds 27-31 (fix-induced findings from the takeover batch, plus what the takeover missed): the markers no longer carry the run secret at all — the fence reads a host-side trust record (per tree: pinned base, build verdict, and the per-file size+ctime inventory), rotated in place on run-identity drift (e1c24ba); the mount-root null is split into outside / unmountable / refused and the gates stay silent where containment cannot exist (f4ab0cf); the pointer round-trip compares the exact owner and refuses planted redirects before resolving (f4ab0cf); the legacy lease path carries no gate authority at all — the mtime bound and its constant are gone, the mirror is advisory and rename-based (dd0b82a); and the launch-dir gate judges where the kernel reports the process standing, not Node's cached spelling (582e5c8). R30-6's confirm request: fixed in ed3c90b.
Verification at this head: the PR's gate suites —
src/commands/review+ the lease suite — 6 038 tests green, 4 skipped (platform gates);tsc --buildclean. The R8-2 witness is Linux-only (APFS/NTFS cannot hold the invalid-UTF-8 name the fixture plants).Reviewer Test Plan
How to verify
From the repository root:
The property itself lives in one file,
packages/cli/src/commands/review/host-execution.canary.test.ts: with a planted repository in place, nothing the pipeline runs on the host may execute out of it. Its oracle is a live plant — a rewritten gitfile naming an admin entry whose owncommondirnames a planted repository carryingfilter.evil.clean, plus a canary file that filter writes on the host — and the fixture is built the way git builds a real one, bare-path backpointer included. Thegitdir:-prefixed backpointer the committed fixtures used is a form git never writes into an admin entry; it fails the round-trip check every route runs first, which turned those fixtures into green statements about the wrong refusal. They now write the bare path.To see the attack by hand:
That last command is exactly what
worktreeResiduereaches once its identity gates pass — which they all do here, the sha pin included, because the plant copies the real admin entry.Evidence (Before & After)
The identity gates that already exist, on the fixture above:
--show-toplevelequals the treerev-parse HEADequals the fetched shagit statuson the hostPWNEDwritten — the plantedfilter.evil.cleanexecutedMutation evidence — eight mutations, eight distinct red sets, baseline green:
comment-statusprobe ungatedfetch-prgate back at step 4 onlyrefuses a poisoned launch directory before the sweep and the fetch(asserts no lease read, no sweep, no git call)repo-contextgate removedrefuses to read identity files through a rewritten review-worktree gitfilelib/git's launch-directory guard neuteredlib/gitcanary and the local-lane (captureLocalDiff) canarybase-treereuse gate removedrevert-hunkgate removedThe canary suite's first case asserts the canary is written when the same command runs ungated, so a fixture that quietly stops being an attack fails there rather than certifying the gates that walked around it. Every refusal case is paired with an admit arm, including a two-layer nested layout that must still measure, resolve and probe normally — a gate that refuses everything passes no test here.
Suites:
src/commands/review+ the lease suite — 119 files, 5 690 passing, 18 skipped.tsc --noEmit,eslint --max-warnings 0andprettier --checkclean on every changed file.Tested on
Environment (optional)
Unit tests under vitest; the reproduction uses only
git. No container runtime required for either.macOS was exercised on this branch's earlier rounds but not on this round's suites. Windows is not a gap that testing would close:
mountRootForrefuses every absolute path there (a drive letter is a colon), so containment cannot exist and the gate never speaks — every containment case is skipped at block level, and the fixtures cannot even be built (a planted name carrying a drive letter mid-path is rejected by NTFS).Risk & Scope
.gitlives under a review temp dir — a checkout cloned into one — is now refused where it was admitted. Nothing this pipeline builds has that shape (every tree it makes is a linked worktree, whose admin entry is outside every layer), and a repository sitting inside the directory reviewed code is handed read-write is not one a host-side command should resolve through anyway.cleanupstill runsworktree pruneandbranch -Dfrom the launch directory ungated. It is the remedy — refusing there would leave a planted tree with nothing able to sweep it — and itsrmSyncclears the path whether or not git can act through the pointer.scratch-treedocuments the same residual, andfetch-prnow asks twice specifically to narrow it rather than to claim it closed. And nothing at all on Windows, for the reason above.comment-statusreports gain aworktreeUntrustedfield (nullon every healthy run) and a distinct warning line, so "the tree is there but its pointer is not trustworthy" is not reported as "run fetch-pr first".Linked Issues
Addresses R19-1 and R1-3 from #9723's review, both disclosed in that PR's description as not closed there, and R9-1 from this PR's own review. Related: #9556.
中文说明
本 PR 做了什么
把宿主可信状态移出 review 沙箱以读写方式绑定挂载的那个目录，并阻止宿主侧 git 经由位于该目录内的指针解析仓库。
三个部分：
.qwen/tmp（被读写挂载）移到.qwen/review-leases（不被任何东西挂载）。为什么需要
#9723 评审里的 R19-1 与 R1-3 是同一个根因：流水线事后信任的状态，存放在它交给容器读写的那个目录里。
两条都不是沙箱打开的。在今天的
main上，被审仓库的命令以宿主用户身份、在毫无文件系统限制的情况下运行，因此那里同样够得到、而且够得更宽；容器化收窄了可达范围，而非制造了它。但它确实没有关掉这两条，而一个声称容器化的required不应把宿主可信状态留在可写面之内。Lease。
cleanupReviewWorktreeLeases仅凭(sessionId, promptId, repositoryRoot)匹配 —— 没有任何来源校验 —— 随后对 lease 中所写的工作树与分支执行强制删除。这些文件此前就在 review 临时目录里。被审代码修改其中一个，就能让另一个会话的清理销毁错误的树；植入一个带外来 session id 的 lease，则无人会清扫，该 PR 在这台机器上被永久卡住，直到有人手工删除该文件。Gitfile。 流水线树的
.git无法移动 —— git 要求它位于<tree>/.git—— 所以移动的是检查。已有的身份门看不见这个攻击，而且本来就不可能看见：写入者同时选择了 gitfile 的指向与该指向处的 backpointer，因此往返自洽；--show-toplevel仍然打印 gitfile 所在的那棵树；全程不涉及符号链接；而向 git 询问 common dir，其解析经过被改写的 gitfile，因此那个答案出自同一只手。位置才是有诚实答案的问题：真实 linked worktree 的管理条目位于<repo>/.git/worktrees/之下、在挂载之外，而植入的那个必须在挂载之内。为什么这道门必须在更多地方被问（R9-1）。 第一版只在写入处问了它 —— 即被植入的
filter.<x>会转化为宿主执行的那些 checkout —— 别处一概没问。一个逐调用点关闭的类别，会在下一个有人新增的调用点重新打开，而它确实打开了：worktreeResidue（scratch-tree、agent-prompt）git status，它会刷新索引，而刷新会运行仓库配置的clean过滤器fetch-pr第 1–2 步cleanStale的强制删除与 PRgit fetch，两者都从process.cwd()发现仓库process.cwd()跑 git 的命令 ——load-rules、submit、match-remote、run、revert-hunk、cleanupshow <base>:<path>）、提交目标、被 fetch 的远端comment-statusgit -C <review worktree>探针changedSinceComment: false来自 PR 作者植入的仓库 —— 流水线由此认证「某条阻塞线程背后的代码没有变动」repo-context--git-common-dir、cat-file -e、ls-tree、show <base>:<path>captureLocalDiffrev-parse --show-toplevel，随后git diff（索引刷新；diff.<driver>.command）base-tree复用快路径.git解析的rev-parse HEAD—— 而且它直接返回，因此重建之前的那道门对它从不运行revert-hunkcheck-attr与ls-files—— 此处无执行，git apply不跑过滤器.qwen/tmpmountRootFor自己记录的嵌套几何里，外一层的植入位于内层根之外、外层 review 读写挂载之内，被所有门放行启动目录那几行是一个问题而不是七个，修法也是一次：
lib/git的每个包装器跑 git 时都不带cwd，因此它们全部从process.cwd()发现仓库。于是这个问题被放到那里问 —— 一个文件里的四个函数 —— 而不是在每个恰好跑 git 的命令里各问一次。会抛的包装器抛；gitProbe改为返回它自己文档里定义的「根本无法运行」值，因为那正是 probe 的契约、每个gitOpt调用方本来就处理它。releaseWorktree说明了这个划分为何正确：经由被污染的启动目录，它的worktree remove被拒绝，而其后的rmSync仍然清掉该路径 —— 补救手段照常工作，同时 git 不再经由植入物动作。fetch-pr之上保留自己的门，因为它在发出第一条 git 命令之前就会改动非 git 的状态：lease 的读与写。包含判定那一行需要格外小心。挂载必须保持最内层 —— 取第一次出现会把绑定挂载放宽到外层临时目录，把
<repo>/.git和所有兄弟检出都拉进容器。但「哪个目录可能被以读写方式交给容器」与「被审代码可能已经写过哪里」是两个不同的问题，在嵌套布局下它们有不同的答案。只有不信任判定放宽，放宽到最外层。这对诚实布局零成本：在 linked worktree 内部运行的git worktree add，其管理条目仍然写在主仓库的.git/worktrees/下 —— 位于每一层之外，而不只是位于最深一层之外。该性质已写入用例，因此若未来的 git 改变这一行为，会变红而不是悄悄拒绝每一个嵌套 review。mountRootFor从sandboxed-exec移入worktree（原位置改为再导出，所有导入方不变），从而让这些门可以默认使用它，而不必让每个新调用点都记得把它作为参数传进来。这正是使这次收口成为结构性关闭、而非又一轮逐点打补丁的原因。评审者验证方案
如何验证
在仓库根目录执行：
性质本身集中在一个文件里：
packages/cli/src/commands/review/host-execution.canary.test.ts—— 存在植入仓库时，流水线在宿主上运行的任何东西都不得从其中执行。它的判据是一个真实可用的植入 —— 被改写的 gitfile 指向一个管理条目，该条目自己的commondir又指向一个携带filter.evil.clean的植入仓库，外加一个该过滤器会在宿主上写出的 canary 文件 —— 并且夹具按 git 真实构造的方式搭建，包括裸路径 backpointer。已提交夹具此前写的gitdir:前缀 backpointer 是 git 从不写入管理条目的形式；它会在所有路由最先执行的往返校验处失败，从而把那些夹具变成「关于错误拒绝原因」的绿色断言。现已改为写裸路径（手工复现脚本见上方英文部分）。上述脚本最后一条命令，正是
worktreeResidue在其身份门全部通过后会执行的那一条 —— 而在这里它们确实全部通过，包括 sha 钉定，因为植入物复制了真实的管理条目。证据（前后对比）
已有身份门在上述夹具上的表现：
--show-toplevel等于该树rev-parse HEAD等于 fetched shagit statusPWNED—— 植入的filter.evil.clean被执行变异证据 —— 八个变异，八组互不相同的红，基线全绿：
comment-status探针去门fetch-pr的门退回只在第 4 步refuses a poisoned launch directory before the sweep and the fetch（断言没有 lease 读取、没有清扫、没有任何 git 调用）repo-context的门refuses to read identity files through a rewritten review-worktree gitfilelib/git的启动目录守卫失效lib/gitcanary 与本地车道（captureLocalDiff）canarybase-tree复用门revert-hunk门canary 套件的第一条用例断言：同一条命令在无门情况下运行时 canary 会被写出 —— 因此一个悄悄不再构成攻击的夹具会在那里失败，而不是去认证那些绕过了它的门。每条拒绝用例都配有放行臂，其中包括一个两层嵌套布局，它必须仍能正常测量、解析与探测 —— 一个「拒绝一切」的门在这里通不过任何用例。
套件：
src/commands/review加 lease 套件 —— 119 个文件、5 690 通过、18 跳过。所有改动文件的tsc --noEmit、eslint --max-warnings 0、prettier --check均干净。测试平台
运行环境（可选）
单测在 vitest 下运行；复现脚本只用到
git。两者都不需要容器运行时。macOS 在本分支更早的轮次里跑过，但本轮套件未在其上运行。Windows 不是靠补测试就能补上的缺口：
mountRootFor在那里拒绝每一个绝对路径（盘符就是冒号），因此容器化无法存在、这道门根本不会开口 —— 所有容器化用例按 block 整体跳过，而且夹具在那里根本造不出来（路径中段带盘符会被 NTFS 拒绝）。风险与范围
.git位于 review 临时目录之下的仓库 —— 即被克隆进该目录的检出 —— 现在会被拒绝，而此前是被放行的。本流水线构建的东西没有这种形状（它创建的每棵树都是 linked worktree，其管理条目在每一层之外），而一个位于「被审代码可读写目录」内部的仓库，本来也不该成为宿主侧命令解析的对象。cleanup仍然从启动目录无门地执行worktree prune与branch -D。它是补救手段 —— 在那里拒绝会让植入的树无人可扫 —— 而它的rmSync无论 git 能否经由该指针动作，都会清掉路径。scratch-tree记录了同样的残留，而fetch-pr现在问两次，正是为了收窄它，而非声称关闭它。以及 Windows 上完全不生效，理由同上。comment-status报告新增worktreeUntrusted字段（健康运行时恒为null）与一条独立的告警行，使得「树在，但它的指针不可信」不会被报成「请先跑 fetch-pr」。关联 Issue
处理 #9723 评审中的 R19-1 与 R1-3（两者都已在该 PR 描述中披露为「本 PR 关不掉的」），以及本 PR 自身评审中的 R9-1。相关：#9556。
更新（2026-09-08，head 582e5c8）—— 第 26 轮未决 Critical
第 26 轮全部 18 条未决 Critical 已在八个提交中修复，每条都带 finding 指定的 fix-witness 并实际执行了变异校验（移除新增分支 → witness 变红 → 还原）：
mountRootFor之外用词法包含判定，拼写在内而挂载为空时失败关闭；启动目录 memo 仅以词法扫描为键；Windows 拒绝覆盖无冒号的 UNC 形态；symlink 行走的界按几何区分。releaseWorktree把status: null算作未释放，并在 cwd 被删除时降级而非抛出。isSubpath比较使子目录启动形态保持正确。清扫历史线程时复核了三条更老的 finding，其中两条仍开放并已修复：有损解码分歧（R8-2 —— 3cc9638，git 答案中出现 U+FFFD 即视为未作答）与 prune 臂的第三形态读取（R23-8 —— 4747901）。R23-12 的机制即 R23-2 的回环；对其指定位置有一处刻意的偏离——检查位于
untrustedPointer内但以isSubpath比较而非根相等，这是子目录启动目录的正确推广。全部 134 个评审线程均已逐条回复处置并 resolve。建议级与延后项（测试覆盖、文档）仍记录在评审的 findings 工件中作为后续工作。
第 27-31 轮（针对 takeover 批次的 fix-induced 发现及其遗漏）： marker 不再携带运行密钥——fence 只读宿主侧 trust 记录（每树：钉定的 base、构建结论、逐文件 size+ctime 清单），按运行身份漂移原地轮换（e1c24ba854）；挂载根的 null 被拆分为 outside / unmountable / refused 三种，门在 containment 不存在处保持静默（f4ab0cfafb）；指针回环改为精确属主比较，并在解析前先拒绝被植入的重定向（f4ab0cfafb）；旧路径 lease 完全不具备门禁权威——mtime 界与常量一并移除，镜像是建议性的且经 rename 替换（dd0b82a47d）；启动目录门判定内核报告的进程位置而非 Node 缓存的拼写（582e5c8607）。R30-6 的待确认项已在 ed3c90b 修复。
本 head 的验证： PR 门禁套件 ——
src/commands/review加 lease 套件 —— 6 038 通过、4 跳过（平台门）；tsc --build干净。R8-2 的 witness 仅 Linux（APFS/NTFS 无法持有夹具要植入的非法 UTF-8 名称）。